refactor: introduce SamTag newtype for two-character BAM tag fields

[nh13]

Stacked on #240.

Closes #193

Summary

• Adds SamTag([u8; 2]) to crates/fgumi-sam with named constants for all standard SAM spec tags used by fgumi (RX, QX, MI, CB, CY, BC, QT, OX, BZ, RG) and the fgumi-internal PA tag
• SamTag implements FromStr/TryFrom<&str> (validates length and ASCII), Display, Deref<Target=[u8;2]>, AsRef<[u8;2]>, From<SamTag> for noodles::Tag, and PartialEq<[u8;2]>
• In extract: single_tag and clipping_attribute change from Option<String> to Option<SamTag> — clap now validates tag format automatically via FromStr, eliminating the manual ensure! length checks and String → [u8;2] conversions
• All Tag::new(b'C', b'B') / Tag::new(b'M', b'I') / Tag::new(b'R', b'X') call sites in command files replaced with Tag::from(SamTag::CB/MI/RX)
• unified_pipeline/bam.rs and base.rs default cell tag construction uses SamTag::CB
• downsample.rs local MI_TAG constant replaced by Tag::from(SamTag::MI) inline

Test plan

• cargo ci-test — 2289 tests pass
• cargo ci-fmt — clean
• cargo ci-lint — clean
• Verify --single-tag ABC is rejected by clap before execution
• Verify --single-tag ZU is accepted and used correctly

[codecov]

Codecov Report

❌ Patch coverage is 94.66019% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.29%. Comparing base (f0b83c6) to head (c0b258b).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
src/lib/commands/extract.rs	80.00%	5 Missing ⚠️
src/lib/read_info.rs	50.00%	5 Missing ⚠️
src/lib/unified_pipeline/base.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #241      +/-   ##
==========================================
+ Coverage   89.04%   89.29%   +0.24%     
==========================================
  Files         114      118       +4     
  Lines       55304    57201    +1897     
==========================================
+ Hits        49247    51079    +1832     
- Misses       6057     6122      +65     

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

[nh13]

@coderabbitai review

[nh13]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

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]

Warning

Rate limit exceeded

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

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 8 minutes and 30 seconds.

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

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c88b07f0-dbbd-40c0-84e7-f89ceeb02bdf

📥 Commits

Reviewing files that changed from the base of the PR and between 7216ed1 and c0b258b.

📒 Files selected for processing (33)

• crates/fgumi-consensus/src/duplex_caller.rs
• crates/fgumi-sam/src/builder.rs
• crates/fgumi-sam/src/lib.rs
• crates/fgumi-sam/src/tag.rs
• src/lib/commands/codec.rs
• src/lib/commands/compare/bams.rs
• src/lib/commands/compare/raw_compare.rs
• src/lib/commands/correct.rs
• src/lib/commands/dedup.rs
• src/lib/commands/downsample.rs
• src/lib/commands/duplex.rs
• src/lib/commands/extract.rs
• src/lib/commands/group.rs
• src/lib/commands/review.rs
• src/lib/commands/shared_metrics.rs
• src/lib/commands/simplex.rs
• src/lib/commands/simulate/correct_reads.rs
• src/lib/commands/sort.rs
• src/lib/commands/zipper.rs
• src/lib/grouper.rs
• src/lib/mi_group.rs
• src/lib/read_info.rs
• src/lib/sam/mod.rs
• src/lib/sort/keys.rs
• src/lib/sort/raw.rs
• src/lib/template.rs
• src/lib/unified_pipeline/bam.rs
• src/lib/unified_pipeline/base.rs
• tests/integration/test_bgzf_eof.rs
• tests/integration/test_downsample_command.rs
• tests/integration/test_duplex_command.rs
• tests/integration/test_simplex_command.rs
• tests/integration/test_streaming_input.rs

📝 Walkthrough

Walkthrough

A new SamTag newtype wrapping [u8; 2] was added in crates/fgumi-sam and re-exported at the crate root and via src/lib/sam/mod.rs. SamTag provides const constructors, validation (pattern: ASCII letter then alphanumeric), FromStr/TryFrom<&str>, Display, conversions to noodles::sam::...::Tag, and many predefined tag constants (e.g., CB, RX, MI, RG, PA, AS). The codebase replaced raw two-byte literals and Tag::new() calls with SamTag constants/conversions; Extract CLI fields changed from Option<String> to Option<SamTag> and internal string validation was removed.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	Title accurately describes the main change: introducing SamTag newtype for BAM tag fields.
Description check	✅ Passed	Description clearly explains the SamTag implementation, its usage across modules, and testing approach.
Linked Issues check	✅ Passed	PR fully implements #193 objectives: SamTag newtype with validation, FromStr/Display impls, clap integration, and comprehensive refactoring across all tag-using modules.
Out of Scope Changes check	✅ Passed	All changes are scoped to SamTag introduction and tag field standardization; no unrelated functionality modifications detected.
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
• Commit unit tests in branch 193/nh_samtag-newtype

---

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

src/commands/group.rs (1)

992-994: Consider using SamTag for RX and MI tags as well.

Lines 992 and 994 still use raw byte arrays (*b"RX" and *b"MI"), while line 993 uses the new SamTag::CB. Per PR objectives, SamTag provides constants for both RX and MI. Using them consistently would improve type safety and readability.

♻️ Proposed refactor for consistency

-        let raw_tag: [u8; 2] = *b"RX";
+        let raw_tag: [u8; 2] = *SamTag::RX;
         let cell_tag = Tag::from(SamTag::CB);
-        let assign_tag_bytes: [u8; 2] = *b"MI";
+        let assign_tag_bytes: [u8; 2] = *SamTag::MI;

🤖 Prompt for AI Agents

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

In `@src/commands/group.rs` around lines 992 - 994, Replace the raw byte arrays
for RX and MI with SamTag-based Tags for consistency: instead of let raw_tag:
[u8; 2] = *b"RX" and let assign_tag_bytes: [u8; 2] = *b"MI", construct Tag
values using Tag::from(SamTag::RX) and Tag::from(SamTag::MI) (similar to how
cell_tag uses Tag::from(SamTag::CB)); update the variable types if necessary to
hold a Tag rather than a [u8;2].

src/commands/review.rs (1)

686-686: Inconsistent MI tag construction.

Lines 627 and 835 use Tag::from(SamTag::MI), but lines 686 and 795 still use raw byte form Tag::from([b'M', b'I']). For consistency with the PR objective, consider updating these as well.

♻️ Proposed fix

-            let mi_tag = noodles::sam::alignment::record::data::field::Tag::from([b'M', b'I']);
+            let mi_tag = noodles::sam::alignment::record::data::field::Tag::from(SamTag::MI);

Apply at both line 686 and line 795.

Also applies to: 795-795

🤖 Prompt for AI Agents

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

In `@src/commands/review.rs` at line 686, The mi_tag is constructed
inconsistently: replace the raw byte construction using
noodles::sam::alignment::record::data::field::Tag::from([b'M', b'I']) with the
canonical Tag::from(SamTag::MI) to match other usages; update both occurrences
where mi_tag is defined so they use SamTag::MI (ensure you import or reference
SamTag accordingly and keep the variable name mi_tag and the same surrounding
logic).

src/commands/extract.rs (1)

763-764: Optional consistency improvement. These local byte literals could use SamTag constants (e.g., &*SamTag::RX) for self-documentation, aligning with the PR's typed-tag goal. Low priority.

🤖 Prompt for AI Agents

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

In `@src/commands/extract.rs` around lines 763 - 764, Replace the hard-coded byte
literals assigned to umi_tag and cell_tag with the typed SamTag constants to
improve self-documentation: use the existing SamTag constants (e.g., SamTag::RX
and SamTag::CB) and take a byte-slice reference compatible with the current
signature (for example by using &*SamTag::RX and &*SamTag::CB) when initializing
umi_tag and cell_tag so the variables remain of type &[u8; 2] while aligning
with the PR’s typed-tag approach.

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@crates/fgumi-sam/src/tag.rs`:
- Around line 50-58: The public const constructor SamTag::new currently accepts
any bytes and can be used to create invalid SamTag values; update it to validate
both bytes are printable ASCII (0x20..=0x7E) by adding a const-compatible
assertion that checks a and b are each within that range and includes a
descriptive message, so invalid inputs fail at compile time in const contexts
and panic at runtime otherwise while keeping the function const, pub, and
#[must_use].

---

Nitpick comments:
In `@src/commands/extract.rs`:
- Around line 763-764: Replace the hard-coded byte literals assigned to umi_tag
and cell_tag with the typed SamTag constants to improve self-documentation: use
the existing SamTag constants (e.g., SamTag::RX and SamTag::CB) and take a
byte-slice reference compatible with the current signature (for example by using
&*SamTag::RX and &*SamTag::CB) when initializing umi_tag and cell_tag so the
variables remain of type &[u8; 2] while aligning with the PR’s typed-tag
approach.

In `@src/commands/group.rs`:
- Around line 992-994: Replace the raw byte arrays for RX and MI with
SamTag-based Tags for consistency: instead of let raw_tag: [u8; 2] = *b"RX" and
let assign_tag_bytes: [u8; 2] = *b"MI", construct Tag values using
Tag::from(SamTag::RX) and Tag::from(SamTag::MI) (similar to how cell_tag uses
Tag::from(SamTag::CB)); update the variable types if necessary to hold a Tag
rather than a [u8;2].

In `@src/commands/review.rs`:
- Line 686: The mi_tag is constructed inconsistently: replace the raw byte
construction using
noodles::sam::alignment::record::data::field::Tag::from([b'M', b'I']) with the
canonical Tag::from(SamTag::MI) to match other usages; update both occurrences
where mi_tag is defined so they use SamTag::MI (ensure you import or reference
SamTag accordingly and keep the variable name mi_tag and the same surrounding
logic).

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ab9a1feb-7828-4236-809e-b9ebb72023db

📥 Commits

Reviewing files that changed from the base of the PR and between e71345e and 3c30430.

📒 Files selected for processing (15)

• crates/fgumi-sam/src/lib.rs
• crates/fgumi-sam/src/tag.rs
• src/commands/codec.rs
• src/commands/correct.rs
• src/commands/dedup.rs
• src/commands/downsample.rs
• src/commands/duplex.rs
• src/commands/extract.rs
• src/commands/group.rs
• src/commands/review.rs
• src/commands/shared_metrics.rs
• src/commands/simplex.rs
• src/lib/sam/mod.rs
• src/lib/unified_pipeline/bam.rs
• src/lib/unified_pipeline/base.rs

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (1)

crates/fgumi-sam/src/builder.rs (1)

600-609: These builders still bypass the new invariant for custom tags.

The built-in RG path now uses SamTag, but custom attrs still come through raw &str and are converted with a byte-length check only. That means invalid tags are still silently dropped or emitted from arbitrary 2-byte UTF-8, so the public builder API still doesn't guarantee a valid SAM tag. Prefer storing SamTag here too, or validate via SamTag::try_from before insert.

Based on learnings, in the fgumi-sam crate (crates/fgumi-sam), the builder module is intentionally public API.

Also applies to: 654-661, 835-844

🤖 Prompt for AI Agents

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

In `@crates/fgumi-sam/src/builder.rs` around lines 600 - 609, The custom-attribute
path currently builds raw two-byte Tags from &str (attrs) which can accept
invalid or arbitrary UTF-8; change this to validate/convert the attribute key
via SamTag::try_from (or store SamTag in the builder) before inserting: for each
(tag_str, value) in self.attrs, call SamTag::try_from(tag_str) and on success
convert to Tag (Tag::from(sam_tag)) and insert that, otherwise handle the
error/skip accordingly; apply the same change to the other occurrences mentioned
(the blocks around first_read.data_mut().insert(...) at the other ranges).

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@crates/fgumi-sam/src/tag.rs`:
- Around line 4-6: Update the module docs to stop calling SamTag::new
"infallible" and instead state that SamTag::new will panic on invalid bytes;
keep the distinction that std::str::FromStr is the fallible (non-panicking)
constructor used by clap for CLI parsing. Reference SamTag::new and
std::str::FromStr in the doc text so readers know which constructor panics and
which returns a Result.
- Around line 60-65: The SamTag constructor currently allows any printable
ASCII; tighten validation in SamTag::new (and the other tag constructor at the
same file, e.g., the second const constructor around lines 171-175) to enforce
the SAM spec: require the first byte to satisfy is_ascii_alphabetic() and the
second to satisfy is_ascii_alphanumeric(); replace the existing
is_ascii_graphic() checks with a.is_ascii_alphabetic() &&
b.is_ascii_alphanumeric(), and apply the same check in the other constructor so
invalid tags like "A!" or "1A" are rejected.

In `@src/lib/commands/extract.rs`:
- Around line 531-536: The current guard only rejects RX but needs to reject all
reserved/always-appended tags (e.g., RG, RX, CB, CY, BC, QT, QX) and re-apply
the SAM aux-tag syntax validation that was lost; update the check around
self.single_tag in extract.rs to validate that the provided tag is not one of
the reserved tags (RX, RG, CB, CY, BC, QT, QX) and also validate the aux-tag
syntax (the same rules SamTag previously enforced beyond ASCII) — either by
calling a new SamTag::is_valid_aux_tag(s) helper or by expanding
SamTag::from_str to return a clear error for bad aux-tag syntax and using that
here; ensure the ensure! message mentions which reserved tag was rejected and
includes the invalid tag string for debugging.

---

Nitpick comments:
In `@crates/fgumi-sam/src/builder.rs`:
- Around line 600-609: The custom-attribute path currently builds raw two-byte
Tags from &str (attrs) which can accept invalid or arbitrary UTF-8; change this
to validate/convert the attribute key via SamTag::try_from (or store SamTag in
the builder) before inserting: for each (tag_str, value) in self.attrs, call
SamTag::try_from(tag_str) and on success convert to Tag (Tag::from(sam_tag)) and
insert that, otherwise handle the error/skip accordingly; apply the same change
to the other occurrences mentioned (the blocks around
first_read.data_mut().insert(...) at the other ranges).

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e665b9a3-71f9-4599-b6f5-16e08c388bf0

📥 Commits

Reviewing files that changed from the base of the PR and between 3c30430 and 8c3d7be.

📒 Files selected for processing (33)

• crates/fgumi-consensus/src/duplex_caller.rs
• crates/fgumi-sam/src/builder.rs
• crates/fgumi-sam/src/lib.rs
• crates/fgumi-sam/src/tag.rs
• src/lib/commands/codec.rs
• src/lib/commands/compare/bams.rs
• src/lib/commands/compare/raw_compare.rs
• src/lib/commands/correct.rs
• src/lib/commands/dedup.rs
• src/lib/commands/downsample.rs
• src/lib/commands/duplex.rs
• src/lib/commands/extract.rs
• src/lib/commands/group.rs
• src/lib/commands/review.rs
• src/lib/commands/shared_metrics.rs
• src/lib/commands/simplex.rs
• src/lib/commands/simulate/correct_reads.rs
• src/lib/commands/sort.rs
• src/lib/commands/zipper.rs
• src/lib/grouper.rs
• src/lib/mi_group.rs
• src/lib/read_info.rs
• src/lib/sam/mod.rs
• src/lib/sort/keys.rs
• src/lib/sort/raw.rs
• src/lib/template.rs
• src/lib/unified_pipeline/bam.rs
• src/lib/unified_pipeline/base.rs
• tests/integration/test_bgzf_eof.rs
• tests/integration/test_downsample_command.rs
• tests/integration/test_duplex_command.rs
• tests/integration/test_simplex_command.rs
• tests/integration/test_streaming_input.rs

✅ Files skipped from review due to trivial changes (13)

• src/lib/commands/sort.rs
• src/lib/commands/duplex.rs
• tests/integration/test_bgzf_eof.rs
• tests/integration/test_duplex_command.rs
• src/lib/commands/codec.rs
• src/lib/sam/mod.rs
• src/lib/unified_pipeline/base.rs
• src/lib/commands/compare/bams.rs
• src/lib/commands/review.rs
• src/lib/commands/dedup.rs
• crates/fgumi-consensus/src/duplex_caller.rs
• src/lib/mi_group.rs
• src/lib/commands/downsample.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• src/lib/unified_pipeline/bam.rs

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

src/lib/commands/extract.rs (1)

531-542: Keep the emitted-tag set in one place.

RESERVED_OUTPUT_TAGS and the two write paths now spell out the same tag set separately. The next tag addition can update one site and miss validation or the other output path.

♻️ Refactor direction

const EXTRACT_EMITTED_TAGS: &[SamTag] = &[
    SamTag::RX,
    SamTag::QX,
    SamTag::CB,
    SamTag::CY,
    SamTag::BC,
    SamTag::QT,
    SamTag::RG,
];

Use that constant for the guard, and move the actual tag appends into a small shared helper used by both make_raw_records and make_raw_records_static.

Also applies to: 812-843, 1149-1180

🤖 Prompt for AI Agents

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

In `@src/lib/commands/extract.rs` around lines 531 - 542, Create a single shared
constant (e.g., EXTRACT_EMITTED_TAGS: &[SamTag]) containing the tags currently
duplicated in RESERVED_OUTPUT_TAGS, then replace the local RESERVED_OUTPUT_TAGS
usage in the guard with that constant; next factor the logic that appends those
emitted tags out of make_raw_records and make_raw_records_static into a small
shared helper (e.g., append_extract_emitted_tags or build_emitted_tags) and call
it from both make_raw_records and make_raw_records_static so both validation
(the ensure! guard) and the actual output paths reference the same canonical tag
set (update references to RESERVED_OUTPUT_TAGS accordingly).

src/lib/commands/duplex.rs (1)

548-548: Consider removing remaining raw b"CB" literals for one source of truth.

You now derive Tag from SamTag::CB, but raw grouper paths still hardcode *b"CB". Reusing a shared derived byte pair would prevent drift.

Suggested cleanup

-        let cell_tag = Tag::from(SamTag::CB);
+        let cell_tag = Tag::from(SamTag::CB);
+        let cell_tag_bytes: [u8; 2] = *SamTag::CB.as_ref();

@@
-            .with_cell_tag(Some(*b"CB"));
+            .with_cell_tag(Some(cell_tag_bytes));

@@
-                    .with_cell_tag(Some(*b"CB")),
+                    .with_cell_tag(Some(cell_tag_bytes)),

🤖 Prompt for AI Agents

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

In `@src/lib/commands/duplex.rs` at line 548, You introduced cell_tag via let
cell_tag = Tag::from(SamTag::CB); but there are still raw b"CB" literals in
grouper path code; locate usages of the raw byte pair b"CB" (e.g., in grouper
path construction or pattern matches) and replace them with the shared derived
value (use cell_tag or Tag::from(SamTag::CB).as_ref() / .to_vec() as
appropriate) so all code reads the same source of truth (Tag / SamTag::CB) and
avoids drifting hardcoded byte literals.

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@crates/fgumi-sam/src/tag.rs`:
- Around line 3-7: The doc comment overstates the allowed characters—SamTag
actually enforces that both bytes are ASCII alphabetic characters (A–Z, a–z)
rather than any printable ASCII—so update the rustdoc around the SamTag type
(and the similar text at lines 44–46) to say it wraps [u8; 2] and represents two
ASCII alphabetic characters, and mention use of SamTag::new (const, panics on
invalid bytes) and std::str::FromStr (fallible) for runtime parsing; ensure the
description explicitly lists the exact invariant (two ASCII letters) so it
matches the FromStr/new validation logic.

---

Nitpick comments:
In `@src/lib/commands/duplex.rs`:
- Line 548: You introduced cell_tag via let cell_tag = Tag::from(SamTag::CB);
but there are still raw b"CB" literals in grouper path code; locate usages of
the raw byte pair b"CB" (e.g., in grouper path construction or pattern matches)
and replace them with the shared derived value (use cell_tag or
Tag::from(SamTag::CB).as_ref() / .to_vec() as appropriate) so all code reads the
same source of truth (Tag / SamTag::CB) and avoids drifting hardcoded byte
literals.

In `@src/lib/commands/extract.rs`:
- Around line 531-542: Create a single shared constant (e.g.,
EXTRACT_EMITTED_TAGS: &[SamTag]) containing the tags currently duplicated in
RESERVED_OUTPUT_TAGS, then replace the local RESERVED_OUTPUT_TAGS usage in the
guard with that constant; next factor the logic that appends those emitted tags
out of make_raw_records and make_raw_records_static into a small shared helper
(e.g., append_extract_emitted_tags or build_emitted_tags) and call it from both
make_raw_records and make_raw_records_static so both validation (the ensure!
guard) and the actual output paths reference the same canonical tag set (update
references to RESERVED_OUTPUT_TAGS accordingly).

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0c2ce0eb-6cbd-436f-9922-41e447800ca1

📥 Commits

Reviewing files that changed from the base of the PR and between 8c3d7be and 97d8835.

📒 Files selected for processing (33)

• crates/fgumi-consensus/src/duplex_caller.rs
• crates/fgumi-sam/src/builder.rs
• crates/fgumi-sam/src/lib.rs
• crates/fgumi-sam/src/tag.rs
• src/lib/commands/codec.rs
• src/lib/commands/compare/bams.rs
• src/lib/commands/compare/raw_compare.rs
• src/lib/commands/correct.rs
• src/lib/commands/dedup.rs
• src/lib/commands/downsample.rs
• src/lib/commands/duplex.rs
• src/lib/commands/extract.rs
• src/lib/commands/group.rs
• src/lib/commands/review.rs
• src/lib/commands/shared_metrics.rs
• src/lib/commands/simplex.rs
• src/lib/commands/simulate/correct_reads.rs
• src/lib/commands/sort.rs
• src/lib/commands/zipper.rs
• src/lib/grouper.rs
• src/lib/mi_group.rs
• src/lib/read_info.rs
• src/lib/sam/mod.rs
• src/lib/sort/keys.rs
• src/lib/sort/raw.rs
• src/lib/template.rs
• src/lib/unified_pipeline/bam.rs
• src/lib/unified_pipeline/base.rs
• tests/integration/test_bgzf_eof.rs
• tests/integration/test_downsample_command.rs
• tests/integration/test_duplex_command.rs
• tests/integration/test_simplex_command.rs
• tests/integration/test_streaming_input.rs

✅ Files skipped from review due to trivial changes (19)

• src/lib/sam/mod.rs
• src/lib/commands/sort.rs
• src/lib/read_info.rs
• src/lib/commands/downsample.rs
• tests/integration/test_duplex_command.rs
• tests/integration/test_simplex_command.rs
• src/lib/commands/compare/bams.rs
• src/lib/commands/shared_metrics.rs
• tests/integration/test_streaming_input.rs
• crates/fgumi-sam/src/builder.rs
• src/lib/commands/codec.rs
• src/lib/unified_pipeline/base.rs
• src/lib/sort/keys.rs
• src/lib/sort/raw.rs
• src/lib/commands/simplex.rs
• src/lib/commands/compare/raw_compare.rs
• src/lib/commands/dedup.rs
• tests/integration/test_downsample_command.rs
• src/lib/commands/group.rs

🚧 Files skipped from review as they are similar to previous changes (5)

• src/lib/commands/simulate/correct_reads.rs
• src/lib/grouper.rs
• src/lib/commands/review.rs
• src/lib/template.rs
• src/lib/unified_pipeline/bam.rs

[coderabbitai]

🧹 Nitpick comments (1)

src/lib/commands/duplex.rs (1)

298-299: Complete CB tag migration in grouping calls.

Line 298 and Line 548 use SamTag::CB, but Line 415 and Line 596 still use *b"CB". Prefer one source of truth.

Suggested diff

-            .with_cell_tag(Some(*b"CB"));
+            .with_cell_tag(Some(*SamTag::CB));

-                    .with_cell_tag(Some(*b"CB")),
+                    .with_cell_tag(Some(*SamTag::CB)),

Also applies to: 548-549

🤖 Prompt for AI Agents

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

In `@src/lib/commands/duplex.rs` around lines 298 - 299, Multiple places mix raw
byte literal tags (`*b"CB"`) with the new enum-based tag
(`SamTag::CB`)—standardize to the enum-based Tag by replacing all uses of
`*b"CB"` in grouping/packing calls with `Tag::from(SamTag::CB)` (e.g., where you
set `cell_tag` and in any grouping invocations around the same logic) so there
is a single source of truth; search for `*b"CB"` and update those call sites to
use `Tag::from(SamTag::CB)` to match the existing `cell_tag` usage.

🤖 Prompt for all review comments with AI agents

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

Nitpick comments:
In `@src/lib/commands/duplex.rs`:
- Around line 298-299: Multiple places mix raw byte literal tags (`*b"CB"`) with
the new enum-based tag (`SamTag::CB`)—standardize to the enum-based Tag by
replacing all uses of `*b"CB"` in grouping/packing calls with
`Tag::from(SamTag::CB)` (e.g., where you set `cell_tag` and in any grouping
invocations around the same logic) so there is a single source of truth; search
for `*b"CB"` and update those call sites to use `Tag::from(SamTag::CB)` to match
the existing `cell_tag` usage.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5b701b40-98f7-44bd-aaaf-f2f29d3792ad

📥 Commits

Reviewing files that changed from the base of the PR and between 97d8835 and 7216ed1.

📒 Files selected for processing (33)

• crates/fgumi-consensus/src/duplex_caller.rs
• crates/fgumi-sam/src/builder.rs
• crates/fgumi-sam/src/lib.rs
• crates/fgumi-sam/src/tag.rs
• src/lib/commands/codec.rs
• src/lib/commands/compare/bams.rs
• src/lib/commands/compare/raw_compare.rs
• src/lib/commands/correct.rs
• src/lib/commands/dedup.rs
• src/lib/commands/downsample.rs
• src/lib/commands/duplex.rs
• src/lib/commands/extract.rs
• src/lib/commands/group.rs
• src/lib/commands/review.rs
• src/lib/commands/shared_metrics.rs
• src/lib/commands/simplex.rs
• src/lib/commands/simulate/correct_reads.rs
• src/lib/commands/sort.rs
• src/lib/commands/zipper.rs
• src/lib/grouper.rs
• src/lib/mi_group.rs
• src/lib/read_info.rs
• src/lib/sam/mod.rs
• src/lib/sort/keys.rs
• src/lib/sort/raw.rs
• src/lib/template.rs
• src/lib/unified_pipeline/bam.rs
• src/lib/unified_pipeline/base.rs
• tests/integration/test_bgzf_eof.rs
• tests/integration/test_downsample_command.rs
• tests/integration/test_duplex_command.rs
• tests/integration/test_simplex_command.rs
• tests/integration/test_streaming_input.rs

✅ Files skipped from review due to trivial changes (14)

• tests/integration/test_streaming_input.rs
• tests/integration/test_simplex_command.rs
• src/lib/commands/compare/bams.rs
• src/lib/commands/simplex.rs
• src/lib/commands/sort.rs
• src/lib/commands/codec.rs
• src/lib/unified_pipeline/base.rs
• crates/fgumi-consensus/src/duplex_caller.rs
• src/lib/sort/keys.rs
• tests/integration/test_bgzf_eof.rs
• src/lib/commands/group.rs
• src/lib/commands/correct.rs
• src/lib/template.rs
• src/lib/read_info.rs

🚧 Files skipped from review as they are similar to previous changes (9)

• src/lib/sam/mod.rs
• src/lib/grouper.rs
• src/lib/commands/downsample.rs
• src/lib/commands/compare/raw_compare.rs
• crates/fgumi-sam/src/builder.rs
• src/lib/commands/review.rs
• src/lib/commands/dedup.rs
• src/lib/unified_pipeline/bam.rs
• src/lib/commands/shared_metrics.rs