feat(group,dedup): add --no-umi mode for position-only grouping

[nh13]

Summary

• Add --no-umi flag to both group and dedup commands to enable position-only grouping/deduplication without UMI tags
• Similar to Picard's MarkDuplicates behavior when UMIs are not available

Changes

When --no-umi is specified:

• Strategy is forced to identity (with info log if overriding another strategy)
• UMI validation is skipped (templates without UMI tags are accepted)
• Empty strings are used for UMI grouping (all templates at same position get same MI)
• UMI truncation is skipped
• Error is thrown if combined with --strategy paired

Test plan

• Added unit tests for filter_template with no_umi: true in dedup.rs
• Added integration tests for full workflow in group.rs
• Verified error case when --no-umi used with --strategy paired
• All 2329 tests pass
• Formatting and linting pass

Usage

# Group reads by position only (ignore UMIs)
fgumi group --no-umi -i input.bam -o output.bam

# Mark duplicates by position only (like Picard MarkDuplicates)
fgumi dedup --no-umi -i input.bam -o output.bam

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a no_umi mode to deduplication and grouping commands. New no_umi: bool fields are added to CLI/config structs and threaded through filter, UMI-assignment, and processing functions. In no_umi mode UMI extraction, validation, truncation, and related lookups are skipped; an empty UMI or position-only grouping is used. paired strategy is disallowed, edits are forced to 0, and the effective strategy is set to Identity. Function signatures and call sites now accept no_umi and effective_strategy/effective_edits. Tests, logging, and metrics updated to cover no-UMI behavior.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main feature addition—adding a --no-umi mode for position-only grouping to both group and dedup commands, which directly matches the changeset.
Description check	✅ Passed	The description is directly related to the changeset, explaining the --no-umi flag implementation, behavior, test coverage, and usage examples in detail.
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 docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat/no-umi-mode

---

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

[codecov]

Codecov Report

❌ Patch coverage is 86.96809% with 49 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.36%. Comparing base (ff16591) to head (1dfb847).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
src/commands/dedup.rs	83.55%	37 Missing ⚠️
src/commands/group.rs	92.05%	12 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #56      +/-   ##
==========================================
+ Coverage   83.29%   83.36%   +0.06%     
==========================================
  Files         126      126              
  Lines       50944    51234     +290     
==========================================
+ Hits        42436    42709     +273     
- Misses       8508     8525      +17     

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@src/commands/dedup.rs`:
- Around line 2897-2949: Add a unit test for the raw-byte path that mirrors the
existing RecordBuf tests: create a config from default_filter_config with
config.no_umi = true, a fresh FilterMetrics, and a raw-style template/record
that has no UMI (i.e., no RX tag) and is mapped; then call
filter_template_raw(&raw_template, &config, &mut metrics) and assert it returns
true and increments metrics.accepted_templates to 1. Locate the raw-template
constructor used elsewhere (e.g., create_mapped_template_raw or the
RecordBuf/TemplateRaw builder) to build the no-RX test input, and place the test
near the other no_umi tests so the raw-byte logic at filter_template_raw is
covered.

In `@src/commands/group.rs`:
- Around line 807-810: The #[arg(...)] attribute on the boolean field no_umi
includes a redundant default_value = "false"; remove that default_value so
clap's derive can infer ArgAction::SetTrue for the no_umi bool flag (i.e., leave
#[arg(long = "no-umi")] on the no_umi field or, if you prefer being explicit,
replace default_value with action = ArgAction::SetTrue), ensuring the flag
toggles true when provided.

🧹 Nitpick comments (2)

src/commands/dedup.rs (1)

282-522: filter_template and filter_template_raw are near-duplicates of filter.rs.

Both files carry identical filtering logic (flag checks, MQ tag, UMI validation, no_umi bypass). Every change, like the no_umi additions here, must be mirrored. Consider extracting shared filtering into a common module.

src/commands/group.rs (1)

3075-3193: Good test coverage for no-umi mode.

Covers missing tags, position-only grouping, N-in-UMI acceptance, and paired rejection. Consider adding a multi-threaded no-umi test (e.g. ThreadingOptions::new(2)) to exercise the raw-byte pipeline path, but not blocking.

,

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

🧹 Nitpick comments (1)

src/commands/group.rs (1)

4460-4461: Consider adding a raw byte filter unit test for no_umi: true.

Integration tests cover this path via the pipeline, but a direct filter_template_raw unit test with no_umi: true would strengthen coverage.

🤖 Prompt for AI Agents

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

In `@src/commands/group.rs` around lines 4460 - 4461, Add a unit test that calls
filter_template_raw with a configuration where no_umi: true to verify the
raw-byte filtering branch; create input raw bytes containing a UMI region, call
filter_template_raw (and any helper like TemplateFilter or FilterConfig used in
the diff), and assert the returned bytes match the expected output with the UMI
removed and other bytes unchanged; place the test alongside existing tests for
filter_template_raw and ensure it constructs the same config object (setting
no_umi: true) so the no_umi branch is exercised.

🤖 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/commands/group.rs`:
- Around line 4460-4461: Add a unit test that calls filter_template_raw with a
configuration where no_umi: true to verify the raw-byte filtering branch; create
input raw bytes containing a UMI region, call filter_template_raw (and any
helper like TemplateFilter or FilterConfig used in the diff), and assert the
returned bytes match the expected output with the UMI removed and other bytes
unchanged; place the test alongside existing tests for filter_template_raw and
ensure it constructs the same config object (setting no_umi: true) so the no_umi
branch is exercised.

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 07d82bb and 1dfb847.

📒 Files selected for processing (2)

• src/commands/dedup.rs
• src/commands/group.rs