ci: gate sort correctness against samtools by nh13 · Pull Request #577 · fulcrumgenomics/fgumi

nh13 · 2026-07-12T00:19:14Z

Why

fgumi sort's core contract is that it matches samtools sort — and there's a suite that verifies exactly that: test_sort_correctness (coordinate / queryname-lex / queryname-natural / template-coordinate, in-memory and with disk spills), test_async_reader, and test_sort_write_index. All 18 are #[ignore]d because they shell out to samtools, which is never installed in CI. So fgumi's headline feature has been running on an ungated contract — a sort regression (like the recent simulate template-coordinate bug) could ship green.

This is the "contract with no gate" pattern: reference-parity tests that exist but never run.

What

New sort-parity CI job: installs samtools (apt) and runs the ignored suite via a new ci-test-samtools alias (nextest run --run-ignored ignored-only).
Stacked on fix(simulate): template-coordinate order via canonical fgumi-sort + hermetic tests #576, whose hermetic rewrite removed samtools from the simulate sort tests — so ignored-only now selects exactly these 18 sort-vs-samtools tests (verified: 18 run, 18 pass, ~1s locally).

samtools comes from the runner's apt repo; the tests are tie-insensitive, so they don't depend on a specific samtools patch version (a hard version pin is a possible follow-up if ever needed).

Base

Targets nh/test-simulate-sort-hermetic (#576) and should merge after it — it relies on #576 having un-ignored the simulate sort tests, so ignored-only is exactly the samtools set.

First of a task-list series closing "contract with no gate" gaps (T1).

Summary by CodeRabbit

Tests
- Added a dedicated CI test command that runs the workspace suite while executing only the previously ignored, samtools-gated tests.
- Introduced a CI job that installs samtools, verifies it’s available, and runs the samtools-dependent sort-correctness integration checks for reliable automated test coverage.

coderabbitai · 2026-07-12T00:19:20Z

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: 8c8b484c-3c33-4902-ad53-549eb78b6a3f

📥 Commits

Reviewing files that changed from the base of the PR and between 22fcd9a and 0d5c17e.

📒 Files selected for processing (2)

.cargo/config.toml
.github/workflows/check.yml

Walkthrough

Adds a Cargo alias for ignored samtools-dependent integration tests and a sort-correctness CI job that installs and verifies samtools before running them with nextest.

Changes

Samtools CI validation

Layer / File(s)	Summary
Ignored samtools test pipeline `.cargo/config.toml`, `.github/workflows/check.yml`	Defines `cargo ci-test-samtools` with nextest’s `--run-ignored ignored-only` mode and the `fgumi::integration` binary filter, then adds a read-only CI job that installs Rust tooling, nextest, and samtools before invoking it.

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

Sequence Diagram(s)

sequenceDiagram
  participant GitHubActions
  participant Samtools
  participant CargoAlias
  participant Nextest
  GitHubActions->>Samtools: Install and verify samtools
  GitHubActions->>CargoAlias: Run cargo ci-test-samtools
  CargoAlias->>Nextest: Run ignored fgumi::integration tests

🚥 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 clearly matches the main change: a CI gate for sort correctness using samtools.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 unit tests (beta)

Create PR with unit tests
Commit unit tests in branch nh/ci-samtools-sort-parity

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

codecov · 2026-07-12T00:22:28Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.41%. Comparing base (c018d4f) to head (0d5c17e).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #577      +/-   ##
==========================================
+ Coverage   93.39%   93.41%   +0.02%     
==========================================
  Files         174      174              
  Lines      104122   104122              
==========================================
+ Hits        97244    97268      +24     
+ Misses       6878     6854      -24

☔ 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 · 2026-07-18T11:05:54Z

@coderabbitai review

coderabbitai · 2026-07-18T11:05:58Z

✅ Action performed

Review finished.

coderabbitai

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.cargo/config.toml:
- Around line 6-16: Update the ci-test-samtools alias and its associated
documentation to match the actual validation performed: either add a normalized
comparison against samtools sort output to the referenced integration tests, or
rename the alias/job and comments to describe internal sort-correctness checks
rather than samtools output parity. Preserve the existing samtools-gated test
execution.

In @.github/workflows/check.yml:
- Around line 159-160: Update the actions/checkout step in the workflow to set
persist-credentials to false, preventing the GitHub token from being stored in
local git configuration while preserving the existing checkout behavior.
- Around line 179-181: Add a samtools --version validation step immediately
after the apt-get installation and before the “Run samtools-gated
sort-correctness tests” step, so the workflow fails if samtools is unavailable
on PATH while preserving the existing cargo ci-test-samtools invocation.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 14788f2d-623f-48b3-aaed-dd7cd1980e25

📥 Commits

Reviewing files that changed from the base of the PR and between 47aa253 and 8a13fc4.

📒 Files selected for processing (2)

.cargo/config.toml
.github/workflows/check.yml

nh13 · 2026-07-19T09:23:10Z

@coderabbitai review

coderabbitai · 2026-07-19T09:23:33Z

✅ Action performed

Review finished.

nh13 · 2026-07-19T10:09:05Z

@coderabbitai review

coderabbitai · 2026-07-19T10:09:20Z

✅ Action performed

Review finished.

nh13 · 2026-07-19T13:12:44Z

@coderabbitai review

coderabbitai · 2026-07-19T13:12:50Z

✅ Action performed

Review finished.

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In @.github/workflows/check.yml:
- Around line 158-162: Restrict GITHUB_TOKEN permissions for both added jobs by
adding contents: read under msrv-lockstep in .github/workflows/check.yml lines
158-162 and under sort-correctness in .github/workflows/check.yml lines 181-187.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 23e35c21-908c-49ee-9711-632527925a1c

📥 Commits

Reviewing files that changed from the base of the PR and between 8a13fc4 and 37b339d.

📒 Files selected for processing (2)

.cargo/config.toml
.github/workflows/check.yml

nh13 · 2026-07-19T13:16:05Z

@coderabbitai review

coderabbitai · 2026-07-19T13:16:10Z

✅ Action performed

Review finished.

nh13 · 2026-07-19T14:14:17Z

Addressed the outside-diff finding on GITHUB_TOKEN permissions:

Added permissions: contents: read to the sort-correctness job (the job this PR adds). It only reads the repo — it builds and runs tests and never writes back to GitHub — so the default token scopes are dropped to read-only.
Scope-limited on msrv-lockstep: that job was added in build(msrv)!: honest MSRV in lockstep with the toolchain + let-chain adoption #575 and is already on this PR's base branch, not in this diff. Hardening it (along with the other seven pre-existing jobs in check.yml, none of which set permissions: or persist-credentials: false) belongs in a separate workflow-hardening change rather than being smuggled into a sort-correctness PR.

Also fixed a defect found in pre-push self-review: --run-ignored ignored-only selects every #[ignore]d test in the workspace, and the alias comment claimed those were exactly the samtools-gated ones. They were not — fgumi-consensus has four #[ignore]d regen_* fixture writers (codec_caller.rs, duplex_caller.rs) that are maintainer tools, not correctness gates; each builds a large fixture and regen_write persists a temp BAM via tmp.keep() that nothing cleans up. The alias now scopes the run with -E binary_id(fgumi::integration), which selects exactly the 18 samtools-gated tests (verified locally: 18 run, 18 passed, 258 skipped), and the comment was rewritten to describe what actually bounds the selection.

nh13 · 2026-07-19T15:17:48Z

@coderabbitai review

coderabbitai · 2026-07-19T15:17:55Z

✅ Action performed

Review finished.

`fgumi sort`'s headline guarantee is that it matches `samtools sort` (its own docs claim "1.9x faster than samtools"), and there is a suite that checks exactly that: `test_sort_correctness` (coordinate / queryname-lex / queryname-natural / template-coordinate, in-memory and with disk spills), `test_async_reader`, and `test_sort_write_index`. All 18 are `#[ignore]`d because they shell out to `samtools` — which is never installed in CI, so the guarantee was ungated. A sort regression (e.g. the simulate template-coordinate bug) could ship green. Add a `sort-parity` job that installs samtools and runs the ignored suite via a new `ci-test-samtools` alias (`nextest run --run-ignored ignored-only`). Stacked on the simulate-sort branch, whose hermetic rewrite removed samtools from the simulate tests, so `ignored-only` now selects exactly these 18 sort-vs-samtools tests. They pass in ~1s locally with samtools present.

nh13 · 2026-07-19T16:17:33Z

@coderabbitai review

coderabbitai · 2026-07-19T16:17:41Z

✅ Action performed

Review finished.

nh13 · 2026-07-19T16:38:08Z

@coderabbitai review

coderabbitai · 2026-07-19T16:38:18Z

✅ Action performed

Review finished.

nh13 · 2026-07-19T17:21:29Z

@coderabbitai review

coderabbitai · 2026-07-19T17:21:35Z

✅ Action performed

Review finished.

nh13 · 2026-07-19T20:14:12Z

@coderabbitai review

coderabbitai · 2026-07-19T20:14:33Z

✅ Action performed

Review finished.

nh13 · 2026-07-19T21:13:52Z

@coderabbitai review

coderabbitai · 2026-07-19T21:14:10Z

✅ Action performed

Review finished.

nh13 · 2026-07-19T21:34:39Z

@coderabbitai review

coderabbitai · 2026-07-19T21:34:46Z

✅ Action performed

Review finished.

nh13 temporarily deployed to github-actions July 12, 2026 00:19 — with GitHub Actions Inactive

nh13 mentioned this pull request Jul 12, 2026

ci: compile-gate all features + nightly stress-tests #578

Merged

nh13 force-pushed the nh/ci-samtools-sort-parity branch from 679deb4 to 19c4c26 Compare July 12, 2026 00:32

nh13 temporarily deployed to github-actions July 12, 2026 00:32 — with GitHub Actions Inactive

nh13 force-pushed the nh/test-simulate-sort-hermetic branch from 2360ed8 to 47aa253 Compare July 16, 2026 19:57

nh13 force-pushed the nh/ci-samtools-sort-parity branch from 19c4c26 to 8a13fc4 Compare July 16, 2026 19:57

nh13 temporarily deployed to github-actions July 16, 2026 19:57 — with GitHub Actions Inactive

coderabbitai Bot reviewed Jul 18, 2026

View reviewed changes

Comment thread .cargo/config.toml Outdated

Comment thread .github/workflows/check.yml

Comment thread .github/workflows/check.yml

nh13 force-pushed the nh/test-simulate-sort-hermetic branch from 47aa253 to 829cebd Compare July 18, 2026 15:26

nh13 force-pushed the nh/ci-samtools-sort-parity branch from 8a13fc4 to 37b339d Compare July 18, 2026 15:30

nh13 temporarily deployed to github-actions July 18, 2026 15:31 — with GitHub Actions Inactive

coderabbitai Bot reviewed Jul 19, 2026

View reviewed changes

nh13 force-pushed the nh/ci-samtools-sort-parity branch from 37b339d to 22fcd9a Compare July 19, 2026 14:13

nh13 temporarily deployed to github-actions July 19, 2026 14:14 — with GitHub Actions Inactive

Base automatically changed from nh/test-simulate-sort-hermetic to main July 19, 2026 15:26

nh13 force-pushed the nh/ci-samtools-sort-parity branch from 22fcd9a to 0d5c17e Compare July 19, 2026 15:32

nh13 temporarily deployed to github-actions July 19, 2026 15:32 — with GitHub Actions Inactive

nh13 merged commit 443c2ef into main Jul 19, 2026
13 checks passed

nh13 deleted the nh/ci-samtools-sort-parity branch July 19, 2026 21:54

nh13 mentioned this pull request Jul 19, 2026

chore: release v0.5.0 #454

Merged

This was referenced Jul 21, 2026

fix(sort): reserve Phase-1 input serials under the input lock #600

Merged

test: close spill/cancellation coverage gaps + rstest-ify reject loop (audit E1/E2/E3) #543

Merged

nh13 mentioned this pull request Aug 19, 2026

ci: harden samtools install against apt mirror stalls #825

Merged

Conversation

nh13 commented Jul 12, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What

Base

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

codecov Bot commented Jul 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nh13 commented Jul 18, 2026

Uh oh!

coderabbitai Bot commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nh13 commented Jul 19, 2026

Uh oh!

coderabbitai Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nh13 commented Jul 19, 2026

Uh oh!

coderabbitai Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nh13 commented Jul 19, 2026

Uh oh!

coderabbitai Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

nh13 commented Jul 19, 2026

Uh oh!

coderabbitai Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nh13 commented Jul 19, 2026

Uh oh!

nh13 commented Jul 19, 2026

Uh oh!

coderabbitai Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nh13 commented Jul 19, 2026

Uh oh!

coderabbitai Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nh13 commented Jul 19, 2026

Uh oh!

coderabbitai Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nh13 commented Jul 19, 2026

Uh oh!

coderabbitai Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nh13 commented Jul 19, 2026

Uh oh!

coderabbitai Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nh13 commented Jul 19, 2026

nh13 commented Jul 12, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 12, 2026 •

edited

Loading

codecov Bot commented Jul 12, 2026 •

edited

Loading

coderabbitai Bot commented Jul 18, 2026 •

edited

Loading

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading