Skip to content

build(msrv)!: honest MSRV in lockstep with the toolchain + let-chain adoption - #575

Merged
nh13 merged 2 commits into
mainfrom
nh/msrv-lockstep
Jul 18, 2026
Merged

nh13 merged 2 commits into
mainfrom
nh/msrv-lockstep

Conversation

@nh13

@nh13 nh13 commented Jul 11, 2026 •

Copy link
Copy Markdown
Member

Why

rust-version = "1.87.0" was a false, unverified promise. The dependency floor already requires more (noodles/wide → 1.89, sysinfo → 1.88), and CI builds on the rust-toolchain.toml pin (1.93.0). An MSRV that nothing tests and that consumers can't rely on is worse than none — the same "contract with no gate rots" failure as the doctest/doc-link gaps.

The principled fix is one enforced source of truth: fgumi supports exactly the Rust it develops and tests on.

What

1. rust-version → 1.93.0, matching the rust-toolchain.toml channel (build(msrv)! commit).

2. Adopt let-chains (85 sites). Honestly declaring MSRV ≥ 1.88 means clippy correctly suggests collapsing if cond { if let PAT = expr { … } } into if cond && let PAT = expr { … } (stabilized 1.88). Rather than suppress the lint to hide the MSRV, the code adopts the idiom the MSRV unlocks — applied mechanically via cargo clippy --fix, so the code, the toolchain, and the lints all agree. Rewrites are behavior-preserving (nested-if and && let short-circuit identically).

3. msrv-lockstep CI job (ci: commit) that fails if the two versions drift. Because every other job builds on the rust-toolchain.toml channel, keeping them equal means all of CI already verifies the MSRV compiles — no separate MSRV build job needed.

Marked ! (semver-relevant)

Raising the published MSRV is a breaking change for the crates.io libraries — hence the !. It reflects reality (the crates already didn't build below ~1.89); this makes the manifest honest.

Verification

  • cargo ci-lint (-D warnings -W clippy::pedantic) at 1.93 → clean (was 85 collapsible_if before the migration).
  • cargo ci-test → 4756 passed, 0 failed (behavior preserved).
  • cargo ci-fmt → clean.
  • Lockstep check dry-run: rust-version=1.93.0 == channel=1.93.0 → OK.

Notes

Summary by CodeRabbit

  • Compatibility

    • Raised the minimum supported Rust version to 1.93.0.
    • Updated developer setup guidance to reflect the new toolchain requirement.
  • Build Reliability

    • Added automated checks to ensure the declared minimum Rust version matches the pinned build toolchain, preventing version drift.
  • Maintenance

    • Streamlined internal processing and validation logic without changing command behavior or public interfaces.

@nh13
nh13 temporarily deployed to github-actions July 11, 2026 23:12 — with GitHub Actions Inactive
@coderabbitai

coderabbitai Bot commented Jul 11, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8577b1c4-ea6d-4fbf-9246-84716e0c5593

📥 Commits

Reviewing files that changed from the base of the PR and between f55ac4b and fd0c82d.

📒 Files selected for processing (37)
  • .github/workflows/check.yml
  • CLAUDE.md
  • Cargo.toml
  • crates/fgumi-bam-io/src/prefetch_reader.rs
  • crates/fgumi-consensus/src/codec_caller.rs
  • crates/fgumi-consensus/src/duplex_caller.rs
  • crates/fgumi-consensus/src/filter.rs
  • crates/fgumi-consensus/src/vanilla_caller.rs
  • crates/fgumi-raw-bam/src/noodles_compat.rs
  • crates/fgumi-raw-bam/src/tags.rs
  • crates/fgumi-sam/src/clipper.rs
  • crates/fgumi-sort/src/verify.rs
  • crates/fgumi-sort/src/worker_pool.rs
  • crates/xtask/src/generate_metrics.rs
  • crates/xtask/src/generate_tools.rs
  • docs/DEVELOPING.md
  • src/lib/commands/clip.rs
  • src/lib/commands/codec.rs
  • src/lib/commands/compare/bams.rs
  • src/lib/commands/correct.rs
  • src/lib/commands/dedup.rs
  • src/lib/commands/duplex.rs
  • src/lib/commands/extract.rs
  • src/lib/commands/filter.rs
  • src/lib/commands/group.rs
  • src/lib/commands/merge.rs
  • src/lib/commands/review.rs
  • src/lib/commands/simplex.rs
  • src/lib/mi_group.rs
  • src/lib/template.rs
  • src/lib/umi/parallel_assigner.rs
  • src/lib/unified_pipeline/bam.rs
  • src/lib/unified_pipeline/base.rs
  • src/lib/unified_pipeline/fastq.rs
  • src/lib/unified_pipeline/scheduler/optimized_chase.rs
  • src/lib/validation.rs
  • src/main.rs

Walkthrough

The change raises the Rust baseline to 1.93.0, adds CI enforcement between Cargo and toolchain versions, and applies chained conditional syntax across consensus, command, record, pipeline, scheduling, and parser code without described behavior changes.

Changes

Rust toolchain alignment

Layer / File(s) Summary
Rust version lockstep
.github/workflows/check.yml, Cargo.toml, CLAUDE.md, docs/DEVELOPING.md
Rust requirements are updated to 1.93.0, with CI checking that Cargo.toml and rust-toolchain.toml specify the same version.

Processing and command paths

Layer / File(s) Summary
Record and consensus guards
crates/fgumi-{bam-io,consensus,raw-bam,sam,sort}/**, src/lib/umi/parallel_assigner.rs
Nested option and predicate checks are consolidated across record construction, tags, CIGAR handling, filtering, sorting, and UMI edge discovery.
Command validation and output guards
src/lib/commands/**, src/lib/template.rs, src/lib/validation.rs, src/main.rs
CLI validation, comparison detail collection, reject output, consensus application, MAF extraction, tag propagation, and error handling use chained guards with existing conditions and results.

Pipeline and tooling

Layer / File(s) Summary
Pipeline and scheduling guards
src/lib/unified_pipeline/**, src/lib/commands/group.rs, src/lib/mi_group.rs, crates/fgumi-sort/src/worker_pool.rs
Secondary output handling, queue statistics, worker execution, memory diagnostics, MI flushing, and scheduler priorities are rewritten with equivalent combined conditions.
xtask parser guards
crates/xtask/src/*.rs
Metric implementation, documentation attribute, and tool-description parsing use consolidated pattern matching.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the MSRV bump, lockstep enforcement, and let-chain refactor, and is specific enough for history scanning.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch nh/msrv-lockstep

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jul 11, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 76.88564% with 95 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.88%. Comparing base (f55ac4b) to head (fd0c82d).
⚠️ Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
src/lib/unified_pipeline/bam.rs 38.59% 35 Missing ⚠️
crates/fgumi-consensus/src/codec_caller.rs 25.00% 15 Missing ⚠️
src/lib/unified_pipeline/base.rs 53.12% 15 Missing ⚠️
src/lib/commands/filter.rs 31.25% 11 Missing ⚠️
crates/fgumi-sam/src/clipper.rs 55.55% 4 Missing ⚠️
src/lib/commands/compare/bams.rs 75.00% 4 Missing ⚠️
.../lib/unified_pipeline/scheduler/optimized_chase.rs 25.00% 3 Missing ⚠️
src/lib/commands/clip.rs 81.81% 2 Missing ⚠️
src/lib/commands/duplex.rs 80.00% 2 Missing ⚠️
src/lib/unified_pipeline/fastq.rs 60.00% 2 Missing ⚠️
... and 2 more

❌ Your patch check has failed because the patch coverage (76.88%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #575      +/-   ##
==========================================
+ Coverage   92.84%   92.88%   +0.03%     
==========================================
  Files         166      166              
  Lines      102064   101989      -75     
==========================================
- Hits        94765    94734      -31     
+ Misses       7299     7255      -44     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

nh13 added 2 commits July 16, 2026 16:03
…in and adopt let-chains

The declared `rust-version = "1.87.0"` was never true: the dependency floor
(`noodles`, `wide` -> 1.89; `sysinfo` -> 1.88) already requires more, and CI
builds on the `rust-toolchain.toml` pin (1.93.0). An unverified, false MSRV is a
worse promise than none.

Set `rust-version` to the pinned toolchain (1.93.0), so fgumi supports exactly
the Rust it is developed and tested on -- a single source of truth instead of
two drifting numbers. A follow-on commit adds the CI gate that keeps them equal.

Honestly declaring MSRV >= 1.88 means clippy now (correctly) suggests collapsing
`if cond { if let PAT = expr { .. } }` into let-chains, which stabilized in 1.88.
Adopt them workspace-wide (85 sites, applied via `cargo clippy --fix`) rather
than suppressing the lint: the code, the toolchain, and the lints now agree.
The rewrites are behavior-preserving (nested-if -> `&& let` short-circuits
identically); full test suite green (4756 passed).
Add an `msrv-lockstep` job that fails if `rust-version` in Cargo.toml and the
`channel` in rust-toolchain.toml drift apart. This is what keeps the MSRV claim
honest going forward: neither number can be bumped without the other, and since
every other CI job builds on the rust-toolchain.toml channel, keeping them equal
means all of CI already verifies the MSRV compiles -- no separate MSRV build job
is needed.

Document the policy in CLAUDE.md.
@nh13
nh13 force-pushed the nh/msrv-lockstep branch from 4d1d119 to fd0c82d Compare July 16, 2026 20:04
@nh13
nh13 temporarily deployed to github-actions July 16, 2026 20:04 — with GitHub Actions Inactive
@nh13

nh13 commented Jul 18, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 18, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@nh13

nh13 commented Jul 18, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 18, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@nh13

nh13 commented Jul 18, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 18, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@nh13
nh13 merged commit 0ae197c into main Jul 18, 2026
10 of 11 checks passed
@nh13
nh13 deleted the nh/msrv-lockstep branch July 18, 2026 14:56
nh13 added a commit that referenced this pull request Jul 18, 2026
The Rust 1.93 toolchain adopted in #575 makes clippy's collapsible_if
fire on the nested `if outer_bases_length > 0 { if let Some(outer_qual)
= outer_bases_qual }` in mask_consensus_quals_query_based, failing the
lint job on main (and every PR rebased onto it). Collapse it into a
let-chain, the idiom the newer toolchain unlocks, per the project's
policy of adopting new idioms rather than suppressing the lint.

This branch was previously deployed

1 inactive deployment
github-actions — fd0c82db Deployed Jul 16, 2026 by nh13 via coverage #2609
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