refactor(commands): extract generic verify helper and deduplicate test code

[nh13]

Summary

• Extract verify_sort_order generic function in sort.rs replacing 3 near-identical verification branches (-60 lines)
• Use fgumi_raw_bam::read_name instead of hand-rolled byte offset arithmetic
• Extract read_bam_records test helper in zipper.rs to deduplicate BAM reading boilerplate across 3 test functions

Test plan

• All 1838 tests pass (cargo ci-test)
• Formatting passes (cargo ci-fmt)
• Linting passes (cargo ci-lint)

[codecov]

Codecov Report

❌ Patch coverage is 79.13043% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.74%. Comparing base (3a460ea) to head (8df6675).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
src/commands/sort.rs	77.77%	24 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #152      +/-   ##
==========================================
+ Coverage   83.61%   83.74%   +0.13%     
==========================================
  Files         126      126              
  Lines       51510    51361     -149     
==========================================
- Hits        43069    43012      -57     
+ Misses       8441     8349      -92     

☔ View full report in Codecov by Sentry.
📢 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.

[coderabbitai]

Warning

Rate limit exceeded

@nh13 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 17 minutes and 26 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between e63c8a4 and 8df6675.

📒 Files selected for processing (2)

• src/commands/sort.rs
• src/commands/zipper.rs

📝 Walkthrough

Walkthrough

This pull request refactors verification and test utilities. In sort.rs, a new internal verify_sort_order function and VerifySummary type consolidate duplicated verification logic for multiple sort modes (Coordinate, Queryname, TemplateCoordinate), centralizing record iteration, violation detection, and first-violation name extraction. Verification now initializes the raw reader once and reuses the generic verifier. In zipper.rs, a new read_bam_records test helper replaces repeated inline BAM-reading code across tests. No public APIs or production behavior were changed.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	Title accurately summarizes the main changes: extracting generic verify helper and deduplicating test code across sort.rs and zipper.rs.
Description check	✅ Passed	Description clearly relates to the changeset, detailing the extracted functions, test helper usage, and test plan execution.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch refactor/nh/simplify-fgumi-commands-extract-align

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/commands/zipper.rs (1)

935-948: Consider using record_bufs() iterator for simplicity.

Other test modules (codec.rs, clip.rs) use a more concise pattern:

let records: Vec<_> = reader.record_bufs(&header).collect::<std::io::Result<Vec<_>>>()?;

Also, four test modules now have near-identical read_bam_records helpers. A shared test utility could reduce duplication.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/commands/zipper.rs` around lines 935 - 948, The read_bam_records function
currently manually loops over reader.read_record_buf to collect RecordBufs;
replace this with the noodles::bam reader.record_bufs(&header) iterator and
collect into a Result<Vec<RecordBuf>> (e.g.,
reader.record_bufs(&header).collect::<std::io::Result<Vec<_>>>()?), and update
error handling accordingly in the read_bam_records function; additionally, move
this helper into a shared test utility (used by codec.rs, clip.rs and other
tests) to eliminate near-duplicate implementations so tests call the common
read_bam_records helper instead of duplicating the logic.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/commands/zipper.rs`:
- Around line 934-949: The doc comment block that describes run_zipper is now
attached to the helper read_bam_records; move the read_bam_records function so
it appears after the run_zipper function (or alternatively move/restore the
run_zipper doc comment immediately above run_zipper) so the documentation
applies to run_zipper, ensuring you update the order of the functions
(run_zipper and read_bam_records) and keep the read_bam_records signature fn
read_bam_records(path: &std::path::Path) -> Result<Vec<RecordBuf>> unchanged.

---

Nitpick comments:
In `@src/commands/zipper.rs`:
- Around line 935-948: The read_bam_records function currently manually loops
over reader.read_record_buf to collect RecordBufs; replace this with the
noodles::bam reader.record_bufs(&header) iterator and collect into a
Result<Vec<RecordBuf>> (e.g.,
reader.record_bufs(&header).collect::<std::io::Result<Vec<_>>>()?), and update
error handling accordingly in the read_bam_records function; additionally, move
this helper into a shared test utility (used by codec.rs, clip.rs and other
tests) to eliminate near-duplicate implementations so tests call the common
read_bam_records helper instead of duplicating the logic.

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3a460ea and 5c69f15.

📒 Files selected for processing (2)

• src/commands/sort.rs
• src/commands/zipper.rs