Skip to content

Wave2 polish: clippy/fmt clean, CI, finish dead diagnostic#1

Merged
Peterc3-dev merged 1 commit into
mainfrom
wave2-polish
May 30, 2026
Merged

Wave2 polish: clippy/fmt clean, CI, finish dead diagnostic#1
Peterc3-dev merged 1 commit into
mainfrom
wave2-polish

Conversation

@Peterc3-dev
Copy link
Copy Markdown
Owner

Summary

Polish-to-completion pass on the kv-compressor crate. No changes to the
compression or attention logic — this is lint, test, and CI hygiene plus
finishing one dead diagnostic helper.

Changes

  • Clippy clean (-D warnings) across lib + both binaries:
    • usize::div_ceil / usize::is_multiple_of replacing manual ceil-div and
      % == 0 checks (src/lib.rs, src/bin/realdata-bench.rs)
    • regrouped two unreadable hex RNG-seed literals in tests
    • module-level #![allow(clippy::needless_range_loop)] on the four numeric
      kernels (attention, outlier, grouped, twobit), with a comment
      explaining the rationale: the loop counter indexes packed sign-word slices
      together with magnitude/score/weight arrays in lockstep, and the
      i-outer/d-inner bit-walk; iterator rewrites would obscure the bit-packing
      layout. This is the honest call rather than mangling hot math paths.
  • Finished a dead stub: removed _ensure_v2_view_compiles (a dead
    #[allow(dead_code)] helper sitting after the test module that only
    existed to force GroupedKv::flatten_to_v2 to compile) and replaced it with
    a real unit test, flatten_to_v2_averages_group_mags_and_preserves_outliers.
    Dropped the now-unused CompressedKv import in grouped.rs.
  • Formatting: ran cargo fmt repo-wide (the repo was not fmt-clean, which
    is why the diff is large — most of it is pure reflow).
  • CI: added .github/workflows/ci.yml (stable toolchain) running fmt
    check, clippy --all-targets -D warnings, release build, and tests.

Verification (run locally on this machine — Rust 1.95.0 stable, x86_64-linux)

  • cargo build --release --all-targetspasses
  • cargo test --all-targetspasses, 20 tests (was 19; +1 new test)
  • cargo clippy --all-targets -- -D warningsclean
  • cargo fmt --checkclean
  • cargo run --release --bin kv-bench — runs, reports 6.50x ratio on synthetic
    data as expected

The CI workflow itself has not been run on GitHub Actions yet
(UNVERIFIED until it executes on the PR); it mirrors the exact commands
verified locally above.

Not done / left alone

  • No new benchmark numbers or features were invented; this is a documented
    negative-result research crate and stays that way.
  • realdata-bench still requires externally dumped Qwen3 KV .npz tensors
    (per the Python scripts) to exercise its full path — not run here, no such
    dump on this machine. The synthetic kv-bench path was exercised.
  • criterion remains a dev-dependency with no [[bench]] target wired up
    (pre-existing); left as-is to avoid scope creep.

🤖 Generated with Claude Code

- Resolve all clippy warnings; lib + binaries now pass `cargo clippy
  --all-targets -- -D warnings`:
  - `div_ceil` / `is_multiple_of` for manual ceil-div and modulo checks
    (lib.rs, realdata-bench.rs)
  - regroup two unreadable RNG-seed literals in tests
  - module-level `#![allow(clippy::needless_range_loop)]` on the four
    numeric kernels (attention/outlier/grouped/twobit) where the loop
    counter indexes packed sign-word slices and magnitude/score arrays in
    lockstep; documented why explicit indexing is kept
- Replace the dead `_ensure_v2_view_compiles` helper (was sitting after the
  test module under `#[allow(dead_code)]`) with a real unit test for
  `GroupedKv::flatten_to_v2`, and drop the now-unused `CompressedKv` import
- Normalize formatting repo-wide with `cargo fmt` (repo was not fmt-clean)
- Add GitHub Actions CI (.github/workflows/ci.yml): fmt check, clippy
  -D warnings, release build, and tests on stable

No behavior changes to the compression/attention logic.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Peterc3-dev
Copy link
Copy Markdown
Owner Author

Independent verification — VERDICT: solid, ready to merge

Re-cloned wave2-polish and ran the full pipeline locally (Rust 1.95.0 stable, x86_64-linux). Every claim in the PR body checks out:

Check Claim Result
cargo build --release --all-targets passes PASS (clean)
cargo test --all-targets 20 tests pass (+1 new) PASS — 20 passed, 0 failed
cargo clippy --all-targets -- -D warnings clean PASS — no warnings/errors
cargo fmt --check clean PASS
cargo run --release --bin kv-bench 6.50x ratio on synthetic PASS — exactly 6.50x (1.2 MB vs 8.1 MB q8)

Diff review (452+ / 140-, 9 files)

  • No behavior changes to compression or attention logic — confirmed by reading every hunk in attention.rs, outlier.rs, twobit.rs, grouped.rs. The bulk is pure cargo fmt reflow.
  • The only semantic substitutions are the documented (d+63)/64 → d.div_ceil(64) and d % h == 0 → d.is_multiple_of(h) — both are exact equivalents for the non-negative usizes involved. Verified call sites in lib.rs::sign_words_for, realdata-bench, and the ratio math.
  • Dead-stub replacement is legit: _ensure_v2_view_compiles (which fabricated an empty CompressedKv) is gone; the new flatten_to_v2_averages_group_mags_and_preserves_outliers test exercises the real path. Its arithmetic is correct — group mags 0.15 and 13.25 average to 6.7, and flatten_to_v2 does exactly average the per-group mags and clone signs/outliers through unchanged.
  • The #![allow(clippy::needless_range_loop)] on the four numeric kernels is the honest call given the lockstep sign-word/magnitude bit-walk indexing — better than mangling hot paths.

No overclaims

PR body is honest, including the appropriate caveats: GitHub Actions CI not yet executed (correctly flagged UNVERIFIED), realdata-bench not run (needs external Qwen3 .npz dump), and criterion left unwired to avoid scope creep. The CI workflow mirrors the exact commands verified locally.

No required fixes. Approve / merge.

@Peterc3-dev Peterc3-dev merged commit 1a175ce into main May 30, 2026
1 check passed
@Peterc3-dev Peterc3-dev deleted the wave2-polish branch May 30, 2026 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant