fix(metrics): grouping-key correctness — strand tie-break, unsuffixed MI, library/cell partition, reject duplex-to-simplex (DXM3-02/03/05/07, SIMM3-01)

[nh13]

Summary

Five duplex/simplex-metrics grouping-key correctness fixes. The governing principle: the metrics grouping key must match fgumi's own group/dedup (samtools-style coordinates, strand canonicalization, LB/CB partitioning) — not fgbio blindly. Two audit "parity" claims turned out to be over-reaches and are handled accordingly.

Stacked on #506 (nh/fix-duplex-metrics-empty-half-umi) — the only open PR touching shared_metrics.rs.

DXM3-07 — strand tie-break in the canonical mate-key

The canonical ReadInfoKey ordered mates by (ref, unclipped-5') only, dropping the strand tie-break that fgumi's own group/dedup use ((ref, pos, strand), unified_pipeline/base.rs) and that fgbio's ReadInfo.r1Earlier applies. When both mates of a duplex share an identical (ref, unclipped-5'), the two strands canonicalized to different keys and were reported as two single-strand (ab=1, ba=0) families instead of one (ab=1, ba=1). Added strand to the tuple (<=, matching group).

DXM3-03 — unsuffixed MI counted as a single-strand DS family

An unsuffixed MI (no /A,/B) incremented neither strand counter, so its DS family got ds_size = 0 and was silently dropped by the family-size histogram (which iterates 1..=max), undercounting ds_families and skewing ds_fraction. Now treated as the AB strand (ab=n, ba=0), matching fgbio's Pair(n, 0) single-strand DS family. Root-cause fix — size-0 never occurs.

DXM3-02 — partition families by library (and cell)

ReadInfoKey omitted the library (RG→LB) and cell barcode (CB), so reads from different libraries/cells at the same coordinate/strand merged into one CS/DS family. Added library (via read_info::LibraryIndex, matching group/dedup) + a hardcoded CB to the key, mirroring fgbio's ReadInfo(library, cellBarcode) and its takeNextGroup boundary.

No --cell-tag flag — fgumi deliberately hardcodes CB (group.rs/dedup.rs); fgbio exposes --cell-tag, fgumi does not. This corrects the audit's suggestion.

DXM3-05 — doc-only (intentional samtools divergence)

The unclipped-5′ helper counts soft and hard clips, matching samtools unclipped_start (bam.c, used by markdup + template-coordinate sort) and htsjdk getUnclippedStart, keeping the metrics key consistent with fgumi's own group/dedup coordinates. This is an intentional divergence from fgbio's soft-only unSoftClippedStart. The only defect was the docstring falsely claiming fgbio parity — corrected. No behavior change.

SIMM3-01 — reject duplex input to simplex-metrics

simplex-metrics groups per-UMI by base_umi (MI with /A,/B stripped) and consensus-calls both strands' RX together with no strand-swap. For duplex-UMI input the two strands carry swapped UMI halves (ACGT-TGCA vs TGCA-ACGT), so the merged consensus ties to garbage (NNNN). There is no fgbio oracle here; per the chosen behavior, simplex-metrics now detects a base UMI observed on both the /A and /B strands within a coordinate group and fails loud, pointing at duplex-metrics.

fgbio parity verification

Built a 2-library × (AB+BA) fixture at one coordinate, TemplateCoordinate-sorted, and ran fgbio 4.1.0 CollectDuplexSeqMetrics and fgumi duplex-metrics. Both report identical family_sizes:

family_size	cs_count	ss_count	ds_count
1	0	4	0
2	2	0	2

Two CS families of size 2 (library-partitioned) — confirming DXM3-02.

Tests (TDD, red-first)

• duplex_metrics.rs: test_duplex_strands_cogroup_when_mates_share_five_prime (DXM3-07), test_unsuffixed_mi_counts_as_single_strand_ds_family (DXM3-03), test_families_partitioned_by_library (DXM3-02).
• simplex_metrics.rs: test_simplex_metrics_rejects_duplex_input (SIMM3-01).
• DXM3-07 and DXM3-03 explicitly reverted to confirm RED; all green post-fix, no regressions (2222 tests).

Commits

• 17f4e1a0 — DXM3-03, DXM3-05, DXM3-07 (grouping-key parity)
• cde1627e — DXM3-02, SIMM3-01 (library/cell partition + duplex-input rejection)

Summary by CodeRabbit

• Bug Fixes
  • Corrected duplex/DS family counting to include unsuffixed molecular identifiers.
  • Improved duplex-family grouping when mates share the same 5′ start.
  • Preserved library and cell-barcode context during grouping, including inter-reference pairs.
  • Refined mate ordering to keep duplex strands together.
• Tests
  • Added coverage for duplex-family co-grouping and simplex rejection of duplex-UMI input (with clearer guidance to use duplex-metrics).

[coderabbitai]

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: bf76907e-2e17-4dbc-b34a-71a8356b8ac9

📥 Commits

Reviewing files that changed from the base of the PR and between 8c51b0d and 8cbb9cb.

📒 Files selected for processing (3)

• src/lib/commands/duplex_metrics.rs
• src/lib/commands/shared_metrics.rs
• src/lib/commands/simplex_metrics.rs

---

Walkthrough

Grouping keys now include library and cell-barcode identity, duplex metrics retain unsuffixed MI entries in DS-family counts, and simplex metrics reject inputs containing the same base UMI on both strands.

Changes

Metrics grouping and validation

Layer / File(s)	Summary
Library-aware template grouping src/lib/commands/shared_metrics.rs	ReadInfoKey includes library and cell-barcode fields, and template grouping derives them from BAM metadata with strand-aware mate ordering.
Duplex strand family counting src/lib/commands/duplex_metrics.rs	Non-/B MI entries count toward the AB bucket, with coverage for mates sharing a 5′ start and forming one duplex family.
Simplex duplex-input rejection src/lib/commands/simplex_metrics.rs	Coordinate-group processing detects base UMIs present on both strands, propagates the error, and tests rejection of duplex input.

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

Possibly related PRs

• fulcrumgenomics/fgumi#506: Modifies related duplex-UMI strand assignment and fail-fast validation logic.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main grouping-key and duplex/simplex metrics fixes, including strand tie-breaks, library/cell partitioning, and duplex rejection.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch nh/fix-metrics-grouping-key

---

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

[codecov]

Codecov Report

❌ Patch coverage is 99.60784% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 92.64%. Comparing base (868922c) to head (8cbb9cb).
⚠️ Report is 13 commits behind head on main.

Files with missing lines	Patch %	Lines
src/lib/commands/simplex_metrics.rs	98.79%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #532      +/-   ##
==========================================
+ Coverage   92.59%   92.64%   +0.05%     
==========================================
  Files         166      166              
  Lines       99950   100184     +234     
==========================================
+ Hits        92546    92815     +269     
+ Misses       7404     7369      -35     

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

[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/commands/shared_metrics.rs`:
- Around line 573-576: Update the test suite around
test_families_partitioned_by_library to add coverage for cell_barcode
partitioning: construct two templates with identical coordinate, strand, and
library values but different CB tags, then assert they are assigned to separate
families. Ensure the test exercises the cell_barcode field derived from
SamTag::CB.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8289b2d9-0329-45f6-8ec2-d352217853de

📥 Commits

Reviewing files that changed from the base of the PR and between 00feb04 and 8c51b0d.

📒 Files selected for processing (3)

• src/lib/commands/duplex_metrics.rs
• src/lib/commands/shared_metrics.rs
• src/lib/commands/simplex_metrics.rs

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