Skip to content

fix: satisfy clippy lints added in Rust 1.99 - #302

Merged
panbanda merged 1 commit into
mainfrom
fix/clippy-1.99
Nov 4, 2026
Merged

panbanda merged 1 commit into
mainfrom
fix/clippy-1.99

Conversation

@panbanda

@panbanda panbanda commented Nov 3, 2026 •

Copy link
Copy Markdown
Owner

CI moved to Rust 1.99, whose clippy fails the Lint job on main and on every open PR (#290, #291, #296, #300, #301).

  • assert_is_empty (26 test asserts): assert_eq! against an empty value. Clippy's suggested [] as [T; 0] trips the workspace as_conversions lint, so these compare to Vec::<T>::new().
  • approx_constant: FOUR_BIT_CENTROIDS are Lloyd-Max Gaussian centroids; the +-1.6180 entry only coincides with the golden ratio, so it is allowed with a comment.
  • suboptimal_flops: mul_add in the YaRN frequency blend (DeepSeek-V2, yarn.rs) and the TUI duration formatter. Fused multiply-add, so results can differ in the last bit only.
  • cmd_stop returns 0 once after the match instead of in every arm.

Validation (Rust 1.99.1)

  • cargo clippy --all-targets --all-features: clean
  • cargo fmt --all -- --check: clean
  • cargo test -p higgs -p higgs-engine -p higgs-models -- --test-threads=1: 1384 passed, 0 failed

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • The stop command now consistently returns a success exit code after handling stop outcomes.
  • Improvements
    • YaRN frequency calculations and duration formatting use fused arithmetic, which may slightly affect numerical results.
  • Tests
    • Updated checks to compare results directly with expected empty values; test coverage and expected outcomes remain unchanged.

Rust 1.99's clippy fails CI Lint on main and every open PR:

- assert_is_empty: rewrite emptiness asserts as assert_eq! against an
  empty value so failures show the actual contents. Clippy's suggested
  `[] as [T; 0]` trips the workspace as_conversions lint, so compare to
  Vec::<T>::new() instead.
- approx_constant: the 4-bit TurboQuant table holds Lloyd-Max Gaussian
  centroids; +-1.6180 only coincides with the golden ratio, so allow it.
- suboptimal_flops: use mul_add in the YaRN frequency blend and TUI
  duration formatting.
- match_same_arms tail: cmd_stop returns 0 once after the match.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@open-cla

open-cla Bot commented Nov 3, 2026

Copy link
Copy Markdown

Contributor License Agreement

All contributors are covered by a CLA.

@coderabbitai

coderabbitai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Walkthrough

The pull request updates test assertions to compare expected empty values directly, uses fused multiply-add in several calculations, clarifies a quantization-centroid comment and lint allowance, and moves cmd_stop’s success return after its match.

Changes: Direct Empty-Value Assertions

Layer / File(s) Summary
Update empty-result assertions
crates/higgs-engine/src/*, crates/higgs-models/src/qwen3_*.rs, crates/higgs-models/tests/adapter_registry.rs, crates/higgs/src/metrics_log.rs, crates/higgs/src/model_download.rs, crates/higgs/src/types/openai.rs, crates/higgs/tests/integration/*
Tests now compare empty strings, vectors, and JSON arrays directly with expected values. The tested inputs and expected results remain unchanged.

Numerical Calculation Updates

Layer / File(s) Summary
Update numerical expressions and centroid annotation
crates/higgs-models/src/deepseek_v2.rs, crates/higgs-models/src/yarn.rs, crates/higgs-models/src/turboquant.rs, crates/higgs/src/tui/views/mod.rs
Three calculations use fused multiply-add. The centroid declaration now has a comment identifying the values and a local clippy::approx_constant allowance.

Daemon Stop Return

Layer / File(s) Summary
Return success after stop outcomes
crates/higgs/src/daemon.rs
cmd_stop returns 0 after the match. Existing stop handling and messages remain in place.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Suggested reviewers: dusterbloom

Merge Risk: Minimal · up to b62da

The PR contains lint-driven assertion and numerical updates with no identified behavior or availability risk.

Pre-merge checks: Passed 4 · Inconclusive 1

Failed Failed checks: 1 inconclusive

Check name Status Explanation Resolution
Docstring Coverage Inconclusive Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title "fix: satisfy clippy lints added in Rust 1.99" matches the main change. The PR updates empty-value assertions, adds a local clippy allow for a centroid table, and uses mul_add in YaRN and TU…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title "fix: satisfy clippy lints added in Rust 1.99" matches the main change. The PR updates empty-value assertions, adds a local clippy allow for a centroid table, and uses mul_add in YaRN and TUI duration formatting, all to clear clippy failures after the Rust 1.99 CI move. The title is specific and short, and it does not name every file, which is acceptable.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 4 functions across 17 files. (1 skipped: 1 too large.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 8f8c822 and b62da2c.

Files selected for processing: 18
  • crates/higgs-engine/src/engine.rs
  • crates/higgs-engine/src/mtp.rs
  • crates/higgs-engine/src/reasoning_parser.rs
  • crates/higgs-engine/src/simple.rs
  • crates/higgs-engine/src/tool_parser.rs
  • crates/higgs-models/src/deepseek_v2.rs
  • crates/higgs-models/src/qwen3_moe.rs
  • crates/higgs-models/src/qwen3_next.rs
  • crates/higgs-models/src/turboquant.rs
  • crates/higgs-models/src/yarn.rs
  • crates/higgs-models/tests/adapter_registry.rs
  • crates/higgs/src/daemon.rs
  • crates/higgs/src/metrics_log.rs
  • crates/higgs/src/model_download.rs
  • crates/higgs/src/tui/views/mod.rs
  • crates/higgs/src/types/openai.rs
  • crates/higgs/tests/integration/request_validation.rs
  • crates/higgs/tests/integration/response_contract.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

@panbanda
panbanda merged commit 837f1c2 into main Nov 4, 2026
11 checks passed
@github-actions github-actions Bot mentioned this pull request Nov 4, 2026
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