Skip to content

fix(lint): verify-clippy is red on main from an err_expect, and no PR-time job runs clippy #1283

Description

@inureyes

Problem / Background

main is red under clippy. At 930fd83e, cargo clippy -p mlxcel --lib --tests -- -D warnings exits 101 with a single clippy::err_expect:

error: called `.err().expect()` on a `Result` value
   --> src/multimodal/host_preprocessor_tests.rs:416:10
    |
416 |           .err()
    |  __________^
417 | |         .expect("a graph that cannot admit any image must fail at startup");
    | |_______________^ help: try: `expect_err`
    |
    = note: `-D clippy::err-expect` implied by `-D warnings`

error: could not compile `mlxcel` (lib test) due to 1 previous error

The lint is not feature-specific, which is the correction this issue exists to record. It was first observed under --features cuda on Linux and initially read as a CUDA-only escape, but it reproduces on every feature set that compiles the crate, including the CI feature set on Apple Silicon. Three measurements, all yielding the identical single error at src/multimodal/host_preprocessor_tests.rs:416:10 and could not compile mlxcel (lib test) due to 1 previous error:

Configuration Host Command Result
--features cuda GB10, Linux aarch64, sm_121 cargo clippy --lib --tests --features cuda -- -D warnings exit 101
default features (surgery, no accelerator) same host cargo clippy -p mlxcel --lib --tests -- -D warnings exit 101
--features metal,accelerate self-hosted Apple Silicon, nightly CI make verify-clippy failed 2026-08-21T18:13:56Z, run 32511932812 at dd21ada4

The macOS arm of that table is already reported: nightly-verify filed it into #1256 in the 2026-08-21T19:03:51Z comment, whose gate table reads make verify-clippy = failure, make verify-test = success. #1256 is a rotating tracker that gets reused for every nightly red, so it names the failing target but not the defect, and its body still describes a different, older red (verify-test at 9e2c6675). This issue names the defect and the escape path so the tracker can close.

Current Behavior

Nothing in the code path is cfg-gated, which is why every feature set sees it: pub mod multimodal; at src/lib.rs:29 is unconditional, pub mod host_preprocessor; at src/multimodal/mod.rs:45 is unconditional, #[cfg(test)] #[path = "host_preprocessor_tests.rs"] mod tests; at src/multimodal/host_preprocessor.rs:1393-1395 is unconditional, the test the_default_capacity_is_rejected_with_the_derived_requirement at src/multimodal/host_preprocessor_tests.rs:412-416 carries no cfg, and ensure_xla_image_context_capacity at src/multimodal/host_preprocessor.rs:387-391 returns Result<(), HostPreprocessorError> in every configuration.

The same file has two more .err().expect( call sites that do not fire, and the reason matters before anyone sweeps them: clippy::err_expect only suggests expect_err when the Ok type implements Debug, because Result::expect_err requires T: Debug. Line 416's Ok type is (). Lines 108-109 and 151-152 call load_xla_image_preprocessor, whose Ok type is Option<Box<dyn HostMultimodalPreprocessor>> (src/multimodal/host_preprocessor.rs:172-174) and is not Debug, so those two are silent and cannot be mechanically converted without adding a bound that does not exist. A repository-wide grep finds roughly 30 lines matching .err() followed by .expect(; the overwhelming majority are in this second category.

Introduced by #916 (52b7a5da, merged 2026-08-21T11:12Z), which added the offending test. It has been red on main ever since.

Why it escaped, which is the more useful half of this report. No workflow runs clippy over the general tree at PR time, so nothing could have caught it before merge:

  • .github/workflows/ci.yml has no clippy job at all. Its jobs are changes, deny, fmt, crate-versions, kernel-dtype-keys, mlx-pin, cross-repo-refs, xla-compile. The header comment at .github/workflows/ci.yml:5-9 states the omission is deliberate on runner-cost grounds (chore(ci): move clippy + test from PR-time CI to release pipeline #21, chore(ci): drop clippy+test from release pipeline; keep gate local-only #23), with make verify on the developer's machine as the intended substitute.
  • .github/workflows/pipeline-parallel-ci.yml:187-193 does run cargo clippy -p mlxcel --lib --tests -- -D warnings, on ubuntu-latest with default features, in the two-host-logical job. That is the exact command that reproduces this bug, but the workflow's pull_request trigger is path-filtered to src/distributed/pipeline/** and eight sibling paths (.github/workflows/pipeline-parallel-ci.yml:29-41). feat(xla): support Molmo2 indexed attention pooling #916 touched src/multimodal/**, so the job never ran.
  • .github/workflows/nightly-verify.yml:318-321 runs make verify-clippy once a day at 18:00 UTC (:145-147). It is the only clippy that sees this file, and it did its job: it flagged the failure about seven hours after merge. It is a backstop, not a gate, so main stays red until someone reads [nightly-verify] main is red #1256.
  • Makefile:622-623 records that there is deliberately no verify-clippy-cuda. That comment is accurate about CUDA lint coverage, but it must not be read as the explanation here, because this lint is feature-independent and the metal gate fails on it too.

The lint gate itself is verify-clippy at Makefile:663-666, running cargo clippy --workspace --all-targets --features metal,accelerate -- -D warnings.

Proposed Solution

Two parts. The first is mandatory; the second is a decision the maintainer may resolve as "no change, but record it".

1. Fix the lint. At src/multimodal/host_preprocessor_tests.rs:415-417, replace the two-call form with the single call clippy suggests:

let error = ensure_xla_image_context_capacity(dir.path(), 256, false)
    .expect_err("a graph that cannot admit any image must fail at startup");

This is behavior-preserving: Result::expect_err returns E and panics on Ok with the same message, so the following let HostPreprocessorError::InvalidConfig(message) = &error else block and its three assertions are untouched. Do not silence it with #[allow(clippy::err_expect)]. Leave lines 108-109 and 151-152 alone for the Debug reason above; if a sweep is wanted anyway, it is a separate change and needs the bound added first.

2. Decide the gate, and write the decision into the repository. Options in ascending cost:

  • Cheapest, and the one this report favors: give the existing command a path-unfiltered home. Either widen pipeline-parallel-ci.yml's path filter, or lift cargo clippy -p mlxcel --lib --tests -- -D warnings into a standalone clippy job in ci.yml gated by the existing changes job's Rust filter. It needs no accelerator feature, no self-hosted runner, and no new command: it is already written, already runs on ubuntu-latest, and already reproduces this exact failure. The cost is the mlxcel-core MLX C++ build on a GitHub-hosted runner, which two-host-logical already budgets 45 minutes for (pipeline-parallel-ci.yml:95). This is narrower than make verify-clippy (root package, no --workspace, no --all-targets, no metal,accelerate), so it is a partial gate, not a replacement for the nightly.
  • Or: add a verify-clippy-cuda Makefile target mirroring Makefile:663-666 with --features cuda, so a developer on NVIDIA hardware has a lint target next to verify-test-cuda. This does not address the escape that actually happened (the metal gate already covers this lint), but it closes the separate CUDA-only lint hole that Makefile:622-623 names.
  • Or, if neither is wanted: change nothing in CI, and rewrite the Makefile:622-623 comment so the next person does not read "no verify-clippy-cuda" as the reason a feature-independent lint reached main. State plainly that clippy has no PR-time gate over the general tree and that the nightly is the only backstop.

Rejected: reinstating the full self-hosted make verify at PR time. That is the exact tradeoff #21 and #23 already decided against on runner cost, and .github/workflows/ci.yml:5-9 and .github/workflows/nightly-verify.yml:14-24 both record the reasoning. Do not relitigate it here.

Scope

In scope: src/multimodal/host_preprocessor_tests.rs (one statement at 415-417); whichever gate artifact the decision selects, from .github/workflows/ci.yml, .github/workflows/pipeline-parallel-ci.yml, and Makefile:620-624 plus Makefile:663-666; and CONTRIBUTING.md:59 and docs/installation.md:320-360 if a new Makefile target lands, so the CUDA-specific targets stay discoverable together.

Out of scope: the mlxcel-xla and mlxcel dead-code lint backlog under the XLA feature combination, which .github/workflows/ci.yml:253-259 already records as blocking -D warnings on the xla-compile job; the other .err().expect( call sites whose Ok type is not Debug; and the older verify-test red that #1256's body describes.

Implementation Notes

  • Reuse: cargo clippy -p mlxcel --lib --tests -- -D warnings already exists verbatim at .github/workflows/pipeline-parallel-ci.yml:189-193. Use it rather than composing a new invocation. A new Makefile target, if one lands, mirrors verify-clippy's shape at Makefile:663-666 exactly (same @echo prefix style, same $(CARGO), -D warnings after --).
  • Constraints: any GitHub-hosted clippy job first builds MLX C++ through mlxcel-core, so narrowing the target selector does not make it cheap; .github/workflows/nightly-verify.yml:26-35 spells this out. Do not add a self-hosted PR-time job.
  • Edge cases: clippy's per-crate result cache can hide the failure on a re-run, which is why make verify-clean exists (Makefile:708-712, explained in the comment block at Makefile:652-655). Verify from a cold cache or after touching the file. Under --workspace --all-targets, cargo stops dependent targets once one crate errors, so use --keep-going when checking whether other lints hide behind this one.
  • Error handling: none; this is a compile-time lint with no runtime surface.

Acceptance Criteria

  • cargo clippy -p mlxcel --lib --tests -- -D warnings exits 0 on main on Linux with default features.
  • cargo clippy --workspace --all-targets --features cuda -- -D warnings exits 0 on Linux/NVIDIA.
  • make verify-clippy exits 0 on Apple Silicon, and the next scheduled nightly-verify run reports make verify-clippy = success in its [nightly-verify] main is red #1256 comment.
  • No err_expect diagnostic appears in the output of any of the three runs above, verified from a cold cache (cargo clean first, or make verify-clean on Apple Silicon).
  • The lint is fixed by rewriting the call, not by an #[allow] attribute anywhere in the tree.
  • The gate decision is landed in the repository, not just in this thread: either a workflow job that runs clippy on a PR touching src/multimodal/**, or a Makefile target, or an amended Makefile:622-623 comment that states clippy has no PR-time gate over the general tree.
  • If a workflow job is added, it is demonstrated to fail on a branch that reintroduces the .err().expect() form, so the gate is proven to catch this class rather than merely present.
  • If a Makefile target is added, make help lists it (Makefile:103 area) and CONTRIBUTING.md names it beside make verify-test-cuda.

Verification

# Linux, default features (the command pipeline-parallel-ci.yml already runs)
cargo clippy -p mlxcel --lib --tests -- -D warnings; echo "exit=$?"

# Linux/NVIDIA, full CUDA lint surface; --keep-going so one crate's error does not mask others
cargo clippy --workspace --all-targets --features cuda --keep-going -- -D warnings; echo "exit=$?"

# Apple Silicon, the CI-faithful gate
make verify-clippy; echo "exit=$?"

A pass is exit=0 from each with no err_expect diagnostic in the output. Note that make verify-test-cuda is a test gate, not a lint gate, and passes on the affected tree, so a green CUDA test run is not evidence this is fixed.

Technical Considerations

Found while landing #1276 (PR #1281) on GB10 (Linux aarch64, CUDA sm_121). Confirmed pre-existing rather than introduced there: that branch had a diff of exactly one unrelated file (src/vision/detection/rt_detr_v2/sanitize.rs) after rebasing onto current main, and the failure reproduced on main itself at 930fd83e. make verify-test-cuda on the same tree passed (8229 tests, 0 failed).

The full CUDA lint surface carries nothing else. cargo clippy --workspace --all-targets --features cuda --keep-going -- -D warnings on GB10 at 930fd83e exits 101 with this one error and no other diagnostic, having processed all five workspace members (mlxcel-core, mlxcel-xla, mlxcel-mlx-pin, mlxcel-surgery, mlxcel). So the second acceptance criterion below is one line of work away from passing, not a survey of unknown size.

Toolchain is pinned at rust-toolchain.toml channel = "1.97.1", so the diagnostic is reproducible across checkouts and CI; the local clippy 0.1.97 (8bab26f4f6 2026-07-14) matches what the nightly runner emitted.

Related: #1256 (nightly-verify tracker carrying the macOS observation), #916 (introduced the test), #1048 and Makefile:562-623 (the CUDA test gate and the comment about the missing CUDA lint gate), #21 and #23 (the decision that moved clippy off PR-time CI).

Metadata

Metadata

Assignees

No one assigned

    Labels

    priority:mediumMedium prioritystatus:doneCompletedtype:bugBug fixes, error corrections, or issue resolutions

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions