refactor(bam-io): collapse the duplicated grouping domain types onto one definition (R0b)

[nh13]

Prerequisite for R1c. Discovered while porting steps/group/, which is the first ported code that cannot be written without it.

Why

#734 moved DecodedRecord, GroupKey, GroupKeyConfig, and the library-lookup helpers into fgumi-bam-io, and deliberately left the unified_pipeline / read_info copies in place so nothing had to be rewired. That was the right call at the time: the ported step library only ever produced these values, so two identical definitions coexisted harmlessly.

steps/group/ breaks the arrangement. GroupBam calls TemplateGrouper::add_records, which is fgumi_bam_io::Grouper on one side and the umbrella Grouper on the other — typed over two different DecodedRecords:

error[E0308]: expected `fgumi_bam_io::DecodedRecord`, found `base::DecodedRecord`

Two textually identical definitions are still two types. The call cannot be written until they are one.

What changed

Keep the fgumi-bam-io definition; re-export it from both former homes so every existing crate::unified_pipeline::DecodedRecord and crate::read_info::LibraryIndex path resolves unchanged. Collapsed: DecodedRecord, GroupKey, GroupKeyConfig, LibraryIndex, LibraryLookup, build_library_lookup, unknown_library, and the Grouper trait.

Before removing each copy I diffed the two. All were identical apart from field visibility and doc-comment formatting; where they differed in API the fgumi-bam-io side was already a superset (GroupKeyConfig::name_hash_only, DecodedRecord::{record, raw_bytes_mut, cached_umi_position_opt}, GroupKey::name_hash_only). One exception went the other way and was ported across:

• GroupKey::has_mate_position existed only on the umbrella copy, so it moved to fgumi-bam-io. The surviving definition is now a superset of both.

grouper.rs reached into DecodedRecord's fields directly. Those are private on the fgumi-bam-io copy — made so during #734's review — so five sites now go through into_raw_bytes() / record() / raw_bytes() instead.

Risk

No behavior change; this is deletion plus re-export. Net -483 lines. The suite is unchanged at 8322 passing, which is the main evidence: every caller of these types kept compiling and kept passing against a single definition.

The duplication ledger from the landing plan shrinks accordingly — R6 has this much less to unpick when unified_pipeline is deleted.

Gate

ci-fmt, ci-lint, ci-tag-literals, ci-doc (-D warnings), publish-crates.sh --check, ci-test — all exit 0.

Risk: command output changes none; unsafe changes none and the CLAUDE.md allowlist is unchanged; memory bounds, queue capacities, and thread/backpressure policies change none.

Fix: Consolidate grouping types in fgumi-bam-io and preserve existing import paths through re-exports.

• Resolve incompatible duplicate types used by GroupBam and TemplateGrouper.
• Move GroupKey::has_mate_position into fgumi-bam-io.
• Replace direct DecodedRecord field access with accessors.
• Share unknown_library() across ReadInfo fallbacks.
• Remove 483 lines without behavior changes.
• CI checks and 8,439 tests pass.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 50 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: be889c7b-0b01-4a01-8953-65f2d2f1163d

📥 Commits

Reviewing files that changed from the base of the PR and between 747ad39 and 607a8eb.

📒 Files selected for processing (6)

• crates/fgumi-bam-io/src/grouping.rs
• crates/fgumi-bam-io/src/lib.rs
• src/lib/grouper.rs
• src/lib/read_info.rs
• src/lib/unified_pipeline/bam.rs
• src/lib/unified_pipeline/base.rs

Note

Reviews paused

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

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: f6c69acc-2bee-4f4c-b0ef-5a6de0aa635e

📥 Commits

Reviewing files that changed from the base of the PR and between 410ca1f and c84c92d.

📒 Files selected for processing (1)

• src/lib/read_info.rs

---

Walkthrough

The change centralizes grouping and library APIs in fgumi-bam-io, preserves existing module paths through re-exports, adds mate-position detection, and updates grouper code to use DecodedRecord accessors.

Changes

Shared BAM I/O API migration

Layer / File(s)	Summary
Shared grouping and library exports crates/fgumi-bam-io/src/grouping.rs, crates/fgumi-bam-io/src/lib.rs, src/lib/unified_pipeline/base.rs, src/lib/unified_pipeline/bam.rs	GroupKey, DecodedRecord, GroupKeyConfig, and Grouper now use shared fgumi-bam-io definitions. GroupKey::has_mate_position and unknown_library are publicly available.
Library API path migration src/lib/read_info.rs	Library lookup APIs are re-exported from fgumi-bam-io. ReadInfo::from uses the shared unknown-library fallback. Tests cover declared, unknown, and missing read-group identifiers.
DecodedRecord accessor migration src/lib/grouper.rs	Grouping, MC validation, QNAME comparison, and template construction use into_raw_bytes(), record(), and raw_bytes().

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

Merge Risk: ⚪ Minimal · up to c84c9

This change consolidates duplicate grouping types through re-exports without changing behavior. No actionable merge-blocking risk remains after normal checks and review.

Suggested labels: fgumi group

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title uses valid Conventional Commit syntax, names the affected crate, and accurately describes the consolidation of duplicated grouping types.

---

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

[nh13]

@coderabbitai pause

[coderabbitai]

✅ Action performed

Reviews paused.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main-runall@747ad39). Learn more about missing BASE report.

Additional details and impacted files

@@              Coverage Diff               @@
##             main-runall     #741   +/-   ##
==============================================
  Coverage               ?   94.02%           
==============================================
  Files                  ?      248           
  Lines                  ?   130233           
  Branches               ?        0           
==============================================
  Hits                   ?   122454           
  Misses                 ?     7779           
  Partials               ?        0           

☔ 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]

@coderabbitai review

[coderabbitai]

✅ 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.

[coderabbitai]

Actionable comments posted: 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.

Inline comments:
In `@src/lib/read_info.rs`:
- Around line 20-26: Remove the local UNKNOWN_LIBRARY static and update every
ReadInfo fallback that references it to call the re-exported unknown_library()
function from fgumi_bam_io. Preserve the existing fallback behavior and keep the
public re-export unchanged.

🪄 Autofix

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: 5dac1291-161d-439b-b48f-8007e27eab14

📥 Commits

Reviewing files that changed from the base of the PR and between b0fc25c and 410ca1f.

📒 Files selected for processing (6)

• crates/fgumi-bam-io/src/grouping.rs
• crates/fgumi-bam-io/src/lib.rs
• src/lib/grouper.rs
• src/lib/read_info.rs
• src/lib/unified_pipeline/bam.rs
• src/lib/unified_pipeline/base.rs

[nh13]

Fixed. The local UNKNOWN_LIBRARY static is gone and both ReadInfo fallbacks call the re-exported unknown_library().

Worth naming why this one stings: it is the PR's own thesis violated one level down. The change collapses duplicated types onto a single definition, and I re-exported unknown_library while leaving a second LazyLock<Arc<str>> for the same logical value sitting right beside it — so "unknown" existed as two allocations, one of which build_library_lookup reserves as index 0 and the other of which ReadInfo::from fell back to.

No behavior changed: nothing compares library Arcs by pointer (checked — the only ptr_eq uses in the tree are on sort arenas and pipeline storage, not libraries), so the two allocations always compared equal by value. That is also why there is no new test: the fix is allocation sharing with no observable difference, which is the cosmetic case the review conventions exempt. Flagging that explicitly rather than leaving it as a silent omission.

Sibling audit for the same mistake across the rest of the change: the other statics in the touched files are THREAD_ID_UNSET, NUM_STEPS, and three buffer-size constants — unrelated tuning, not duplicates of anything this PR re-exports. Clean.

The re-export comment now explains the value-level duplication too, so the next reader sees why there is no local static.

8439 tests passing, all static gates green.

[nh13]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

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]

@coderabbitai review

[coderabbitai]

✅ 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]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

No files to review.

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]

@coderabbitai full review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

---

Your included review limit is currently reached under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 50 minutes.