refactor(fgumi): deduplicate core lib modules and remove redundant state

[nh13]

Summary

• Replace duplicated calculate_reference_length in bam_io.rs with fgumi_raw_bam::reference_length_from_raw_bam (-43 lines)
• Replace hand-rolled array/string reversal in tag_reversal.rs with fgumi_sam::reverse_buf_value and revcomp_buf_value (-64 lines)
• Extract shared find_sibling_file helper for find_fai_path/find_dict_path in reference.rs (-16 lines)
• Unify four identical template accessor methods via records_in_range helper, returning &[RecordBuf] instead of Vec<&RecordBuf> (-40 lines)
• Remove redundant base_seq field from ReorderBuffer that was always equal to next_seq (-12 lines)

Test plan

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.65%. Comparing base (3a460ea) to head (78cee20).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #151      +/-   ##
==========================================
+ Coverage   83.61%   83.65%   +0.03%     
==========================================
  Files         126      126              
  Lines       51510    51248     -262     
==========================================
- Hits        43069    42869     -200     
+ Misses       8441     8379      -62     

☔ 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 0 minutes and 37 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 635f60f and 78cee20.

📒 Files selected for processing (5)

• src/lib/bam_io.rs
• src/lib/reference.rs
• src/lib/reorder_buffer.rs
• src/lib/tag_reversal.rs
• src/lib/template.rs

📝 Walkthrough

Walkthrough

This pull request refactors multiple modules to delegate operations to external libraries and consolidate internal helper logic. Changes include: replacing a local reference-length calculation with an external API call in bam_io.rs, centralizing sibling-file lookup logic via a new private helper in reference.rs, renaming base_seq to next_seq for sequence tracking in reorder_buffer.rs, replacing manual tag reversal with external helper functions in tag_reversal.rs, and converting four public template methods from returning vectors to slices in template.rs.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main theme of the PR: refactoring to deduplicate code and remove redundant state across multiple core library modules.
Description check	✅ Passed	The description directly relates to the changeset, detailing each refactoring across five files with line counts and test verification.
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-core-modules

---

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 (2)

src/lib/tag_reversal.rs (1)

174-190: Use rstest for this parameterized case matrix.

Lines 176-189 are parameterized through a manual case loop; convert this to rstest cases to match project test conventions.

As per coding guidelines, **/*.rs: Use rstest for parameterized tests.

🤖 Prompt for AI Agents

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

In `@src/lib/tag_reversal.rs` around lines 174 - 190, The test
test_reverse_buf_value_all_array_types uses a manual loop over test_cases;
convert it to an rstest-parameterized test by replacing the function with a
#[rstest] test that declares parameters (e.g., input, expected) and lists each
tuple as a case(...) for the different array types, then call
fgumi_sam::reverse_buf_value(&input) and assert_eq!(reversed, expected) inside
the test; also add the necessary use rstest::rstest import and remove the Vec
and for loop so each case is reported separately by the test runner.

src/lib/reorder_buffer.rs (1)

288-292: Consider renaming the test helper for clarity.

Line 290 still uses set_base_seq, but Line 291 mutates next_seq. Renaming this helper would better match the new internal model.

🤖 Prompt for AI Agents

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

In `@src/lib/reorder_buffer.rs` around lines 288 - 292, The test helper
set_base_seq in reorder_buffer.rs is misleading because it mutates the internal
field next_seq; rename the function to reflect its purpose (e.g., set_next_seq
or set_sequence_cursor) and update all references in tests to the new name so
the helper accurately describes that it sets the next_seq field used by the
reorder buffer.

🤖 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/lib/template.rs`:
- Around line 341-345: The records_in_range method currently only checks end <=
self.records.len() before slicing, but must also guard that start <= end and
start <= self.records.len(); update records_in_range (function name) to validate
both that start <= end and that start < = self.records.len() (and end <=
self.records.len()) and return an empty slice when the range is invalid so the
slice &self.records[start..end] cannot panic.

---

Nitpick comments:
In `@src/lib/reorder_buffer.rs`:
- Around line 288-292: The test helper set_base_seq in reorder_buffer.rs is
misleading because it mutates the internal field next_seq; rename the function
to reflect its purpose (e.g., set_next_seq or set_sequence_cursor) and update
all references in tests to the new name so the helper accurately describes that
it sets the next_seq field used by the reorder buffer.

In `@src/lib/tag_reversal.rs`:
- Around line 174-190: The test test_reverse_buf_value_all_array_types uses a
manual loop over test_cases; convert it to an rstest-parameterized test by
replacing the function with a #[rstest] test that declares parameters (e.g.,
input, expected) and lists each tuple as a case(...) for the different array
types, then call fgumi_sam::reverse_buf_value(&input) and assert_eq!(reversed,
expected) inside the test; also add the necessary use rstest::rstest import and
remove the Vec and for loop so each case is reported separately by the test
runner.

---

ℹ️ 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 635f60f.

📒 Files selected for processing (5)

• src/lib/bam_io.rs
• src/lib/reference.rs
• src/lib/reorder_buffer.rs
• src/lib/tag_reversal.rs
• src/lib/template.rs