ci: clippy the opt-in feature builds, and cover the CLI - #175
Merged
Conversation
jayhesselberth
force-pushed
the
worktree-crf-perf-punchlist
branch
from
August 1, 2026 15:49
bac8115 to
93af4a1
Compare
The `features` job ran `cargo check`. `RUSTFLAGS: -Dwarnings` only denies
lints that are actually evaluated, and `check` never evaluates a clippy
lint, so an opt-in feature could carry a clippy error indefinitely: the
`features` job did not lint it, and the `clippy` job builds default
features, which do not include `crf-gpu`.
That is not hypothetical. On main today:
cargo check -p escapepod-cli --features crf-gpu -> 0 errors
cargo clippy -p escapepod-cli --features crf-gpu -> 2 errors
(`clippy::large_enum_variant` on `Basecaller`, fixed in the parent
commit.) Switching these steps to clippy closes it; clippy implies
check, so nothing is lost.
Also adds the two coverage holes the same bug sat in:
- `crf-decode` and `crf-gpu` were not exercised at all, despite pulling
tract-onnx, ort and fqxv-align — the same dependency-bump exposure the
job exists for.
- Only the library crates were checked. The CLI is a different crate
from the one each feature flag names and is where the feature-gated
command wiring lives, so `-p escapepod-demux` alone cannot see it.
`escapepod-cli` now gets `cnn-gpu` and `crf-gpu` too.
Timeout 25 -> 35 for the extra steps. Still no CUDA toolkit,
libonnxruntime or GPU required: cudarc and ort load dynamically.
All nine combinations verified green locally.
jayhesselberth
force-pushed
the
worktree-ci-feature-clippy
branch
from
August 1, 2026 15:55
5f7abc1 to
b560b66
Compare
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #174 (which is stacked on #173). Targets
worktree-crf-perf-punchlist, so the diff here is one file.The gap
The
featuresjob rancargo check. The workflow setsRUSTFLAGS: -Dwarnings, but that only denies lints that are actually evaluated — andcargo checknever evaluates a clippy lint. Meanwhile theclippyjob builds default features, which don't includecrf-gpu. So an opt-in feature could carry a clippy error indefinitely with every check green.Demonstrated on
mainas it stands right now:That's
clippy::large_enum_varianton the CLI'sBasecallerenum, which #174 fixes. It has been failing silently for as long ascrf-gpuhas existed.The fix
Switch those steps from
checktoclippy— clippy implies check, so no coverage is lost — and addcomponents: clippyto the toolchain step, which the job didn't request.Then close the two holes the bug was sitting in:
crf-decodeandcrf-gpuweren't exercised at all. They pull tract-onnx, ort and fqxv-align, which is precisely the dependency-bump exposure this job exists to catch (per its own comment, that's how the cudarc and tract-onnx Dependabot PRs slipped through).-p escapepod-demuxstructurally cannot see it.escapepod-clinow getscnn-gpuandcrf-gpualongside the existingmodels-download.Timeout 25 → 35 for the four extra steps. Still needs no CUDA toolkit, no
libonnxruntimeand no GPU — cudarc and ort both load dynamically.Verification
All nine combinations run green locally on this branch:
And the new command is confirmed to fail on unfixed code (the
mainresult above), so this is a guard that actually guards rather than one that happens to be green.Note
I kept the existing style of one
cargo clippyper feature rather than adding--all-targets. Linting tests/benches/examples under each opt-in feature would be additional real coverage — thecrf_basecall_checkexample is feature-gated, for instance — but it roughly doubles the job's work, so it seemed worth deciding separately rather than folding in here.