jouleclaw-loader-gguf: 161 clippy warnings -> 0, and -D warnings now gates it #414

Merged

dcharlot merged 5 commits from fix/loader-gguf-lints into main 2026-10-22 04:30:57 -04:00

Conversation 0 Commits 5 Files changed 44 +550 -512

2026-10-21

Claude

jouleclaw-loader-gguf: 0 warnings, and -D warnings now gates it ...

12 -> 0, in both the default and `rayon fabric-meter` configurations, and
the crate joins the `-D warnings` clippy step in standards-build.yml.

The last dozen needed judgement rather than a script:

- two `from_str` methods that are not FromStr-shaped (one returns Option,
  the other is infallible) renamed to `parse`. Both are file-local, so
  there is no caller churn -- except that my check for callers missed
  mlx.rs, which reaches SafetensorsDtype through a type ALIAS, so
  `SafetensorsDtype::from_str(` never matched. Renaming wants a grep for
  the method name across the workspace, not for the type names you happen
  to know.
- `assert!(FLASH_KV_CHUNK.is_power_of_two())` and `>= 64` moved out of a
  `#[test]` into `const _: () = assert!(..)` beside the constant. These
  are properties of a constant; a test only holds them if someone runs it.
- the `kv_quant_tests` module moved to the end of kv_cache_inplace.rs,
  because 300 lines of real items sat after it.
- an example discarded the `Result` from `matmul_bf16_threaded`, so a
  failure would have been timed as a success.
- `needless_range_loop` allowed per-file in three benches and one test:
  those are separate crates, so src/lib.rs's decision does not reach them.
- checked_div for two guarded divisions, and one unused `mut`.

Two near-misses worth recording:

- `fabric_dispatch.rs` calls `ledger_record` twice with identical
  arguments, which reads exactly like a copy-paste bug. The comment three
  lines above says attention is "recorded as two GEMM-equivalents" so the
  measured µJ/GFLOP table prices it. Deliberate. Left alone.
- clippy's `chunks_exact_mut` suggestion is `as_chunks_mut::<N>().0.iter()`
  -- a non-mutable iterator over a mutable chunk. It does not compile, and
  only the fabric-meter configuration contained one, so the default build
  stayed green while that example was broken. Checking both configurations
  is what caught it.

The gate covers default features only. Under `rayon fabric-meter` this
crate is also clean, but clippy reports jouleclaw-backend-apple through
the path dependency, and that crate has 43 warnings -- 27 `unused_unsafe`
and 15 `unused_imports` that are unused in THAT configuration and needed
in others. Those want cfg work, not deletion, which is the trap #373
already paid for once.

Verified: four lint specs pass, 343 loader-gguf tests, 77 omni tests.

Co-Authored-By: Claude Opus 5 <[email protected]>

f5760cc22d

• View at this point in history

2026-10-21 20:40:32 -04:00

Claude

jouleclaw-loader-gguf: 14 -> 12, the chunks_exact rewrites ...

16 `chunks_exact(N)` -> `as_chunks::<N>().0` across 8 files, which is the
batch that needed the tests rather than the compiler: the two forms differ
in how they expose the trailing remainder, and these walk quantised block
layouts.

The first guard against template suggestions was too crude and rejected
all 28 of these, because `as_chunks::<2>()` legitimately contains angle
brackets. It now rejects only a bare lowercase word in brackets that no
`::` precedes, which is what a placeholder looks like.

One follow-on the suggestion cannot express: `as_chunks` yields
`&[u8; N]`, so the `c.try_into().unwrap()` that `chunks_exact(8)` needed
in onnx.rs became ill-typed. Dropped to `*c`.

Verified: 0 errors, 343 tests pass.

Co-Authored-By: Claude Opus 5 <[email protected]>

2f71fde057

• View at this point in history

2026-10-21 20:32:41 -04:00

Claude

jouleclaw-loader-gguf: 40 -> 14 clippy warnings ...

26 more of clippy's own suggestions: unused_parens (18 of them introduced
by the previous batch's rewrites), collapsible_match, derivable_impls,
manual_range_contains, unnecessary_mut_passed. 0 errors, 343 tests pass.

Co-Authored-By: Claude Opus 5 <[email protected]>

fb295fb281

• View at this point in history

2026-10-21 20:31:17 -04:00

Claude

jouleclaw-loader-gguf: apply clippy's own suggestions, 81 -> 40 ...

`cargo clippy --fix` only applies MachineApplicable suggestions, and most
of this crate's remaining lints carry MaybeIncorrect ones. Those are still
exact byte spans with exact replacement text; "maybe incorrect" means
clippy will not promise the semantics, not that the text is wrong. So
scripts/../apply-suggestions.py reads the JSON and applies them, and the
crate's 343 tests decide.

96 spans across 26 files: collapsible_if, redundant_guards,
needless_late_init, unnecessary_cast, manual_is_multiple_of,
manual_repeat_n, unnecessary_sort_by, manual_slice_size_calculation,
unused_mut, needless_lifetimes, needless_borrows_for_generic_args,
double_parens.

One suggestion class must never be applied this way, and it cost a
revert to learn: `needless_range_loop` emits a *template*, not a
replacement --

    for (i, <item>) in w.iter_mut().enumerate().take(total)

-- leaving both the binding name and the body's `w[i]` uses for a human.
Applied literally it writes `<item>` into the source, which does not
parse; four files failed to compile. They were reverted to HEAD and the
applier now refuses any replacement containing `<` and `>`.

The lesson that actually matters is the ordering: the first batch of 69
was verified green and then left uncommitted while a riskier batch ran on
top, so reverting the breakage could not be done without also losing
verified work. Commit verified progress before the next batch.

Verified: 0 errors, 343 tests pass.

Co-Authored-By: Claude Opus 5 <[email protected]>

fd25cb8b70

• View at this point in history

2026-10-21 20:30:31 -04:00

Claude

jouleclaw-loader-gguf: 161 -> 81 clippy warnings, and what the gate still needs ...

#373 left this crate as the remaining half of the per-configuration lint
hole: `jouleclaw-omni --features metal` is gated with `-D warnings`, but
loader-gguf, which omni depends on, is not linted at all on its own.

Two steps here, neither of them risky:

- `cargo clippy --fix` applied the machine-applicable suggestions. Only
  seven files; most of this crate's lints are not auto-fixable.
- the same three policy decisions omni's lib.rs already records, for the
  same reasons, since this is quantisation and tokeniser code with the
  same shapes: `needless_range_loop` (kernels walk several arrays at one
  index, often at an offset), `too_many_arguments` (a dequantiser's
  signature is its block format), and the two doc-indentation lints.

161 -> 81 on the default feature set.

It still cannot join the `-D warnings` list, and the measurement says
precisely why. Counting distinct sites, deduplicated, by crate:

                              default   + rayon fabric-meter
    loader-gguf src              75              76
    loader-gguf tests/examples   10              14
    jouleclaw-backend-apple       -              43

Three things that measurement settles, all of which I had wrong at some
point while taking it:

- `--no-deps` does NOT exclude workspace path-dependencies. Running
  `clippy -p jouleclaw-loader-gguf` reports jouleclaw-backend-apple's
  warnings too, so gating this crate means gating that one as well.
  That is also why 27 `unused_unsafe` and 15 `unused_imports` appear
  under fabric-meter and not on default: they are backend-apple's, in
  mps_matmul.rs, not this crate's.
- the remainder is in `src`, not tests. I guessed "largely tests and
  examples" from the first number and it is 75 against 10.
- what is left is mechanical rather than policy: 16
  `chunks_exact_to_as_chunks`, 9 `unnecessary_cast`, 9 `redundant_guards`,
  6 `collapsible_if`, 6 `manual_repeat_n`, 5 `manual_is_multiple_of`,
  5 `unnecessary_sort_by`, and a tail of ones and twos.

So closing it is ~80 edits here plus ~43 in a second crate, each needing
care because `chunks_exact` -> `as_chunks` changes remainder handling in
quantisation code. That is a decision about appetite, not a blocker, and
it is left as one.

Verified: 343 loader-gguf tests, the fabric-meter `process_calibration`
build that caught a deleted label yesterday, and omni's own
`-D warnings` gate and 77 tests all still pass.

Co-Authored-By: Claude Opus 5 <[email protected]>

8f4d5b49b1

• View at this point in history

2026-10-21 19:08:08 -04:00