perf: single-pass aux-tag rebuild (RawTagsEditor::rebuild_with) for zipper merge and NM/UQ/MD strip

[nh13]

Summary

Several BAM tag transforms edited a record's auxiliary block with one remove_tag + append_raw_tag per tag, in a loop — and each remove_tag is a fresh O(aux) scan, so K tag operations on a record with M tags cost O(K·M) plus K separate drain/extend splices. This adds RawTagsEditor::rebuild_with(remove, adds): walk the aux once, drop a remove-set, keep the rest in order, append pre-encoded adds, and splice back with a single allocation.

The flagship consumer is fgumi zipper's tag merge, where profiling put the per-tag find_tag_position scans at ~6% of CPU. Because zipper runs concurrently with the aligner (upstream) and sort (downstream) in bwa-mem3 | fgumi zipper | fgumi sort, every CPU-second it doesn't burn is one those neighbors get.

What changed

RawTagsEditor::rebuild_with + TagKeySet + TagBitset (crates/fgumi-raw-bam/src/tags.rs) — the single-pass primitive, plus a TagKeySet membership trait implemented for [[u8; 2]] slices/arrays (small fixed sets) and TagBitset (large per-run sets). TagBitset (a 256×256 bit table) is promoted out of the zipper command into fgumi-raw-bam as the shared membership type. Upsert semantics match the remove_tag(tag); append(tag, ..) idiom exactly, including last-wins de-duplication within adds.

zipper::merge_raw_with (src/lib/commands/zipper.rs) — Step 2 (drop the configured remove-set) and Steps 3–4 (copy unmapped tags) each become one rebuild_with per mapped record instead of a per-tag remove_tag+append loop. Negative-strand reverse/revcomp runs as a separate pass over the appended tags. The remove pass is skipped entirely when no tags are configured for removal (the common case), and the copy pass is skipped when there is nothing to copy — restoring the old no-op behavior for those cases.

regenerate_alignment_tags_raw (crates/fgumi-sam/src/alignment_tags.rs) — the two branches that strip NM/UQ/MD now do one rebuild_with(&[NM, UQ, MD], &[]) instead of three remove calls.

Byte-identity

Output is byte-identical to the old per-tag idiom for all valid input. The zipper rewrite reorders two things, both output-neutral:

• Processing mapped indices in a single strand-agnostic pass — each index is a different record, so cross-record order cannot change any record's bytes.
• Running reverse/revcomp as a separate pass after all tags are appended — each transform locates its tag by key and only reorders bytes within a value, so post-append yields identical bytes.

rebuild_with's contract is pinned by unit tests including an oracle test asserting equality with the naive remove_tag+append loop. An EC2 check (c8g, 20M-record mapped SAM + unmapped BAM) reports 20,000,000 records, 0 content diffs, IDENTICAL before vs after (raw bytes differ only in BGZF block framing).

Not converted (deliberate)

retag apply_op stays on the per-op path: its per-operation metrics (dst_overwritten, src_missing) are counted against record state at each op and ops apply left-to-right, so batching would mis-count overwrites; each op also touches only 1–2 tags, so there is no K-scan blowup. Value-update families (copy_umi, correct, dedup MI), template.rs MQ/MC/MS (mixed update/remove + signed/unsigned encodings), clipper per-base truncation, pure in-place reverse/revcomp, and fresh SamBuilder builds are different primitives and left alone.

Verification

• cargo ci-test (full workspace): 10,204 passed, 0 failed, 31 skipped.
• cargo ci-fmt / cargo ci-lint (pedantic, -D warnings) / cargo ci-doc (-D warnings) / tag-literals / publish-order: green.
• New tests: rebuild_with contract (drop / keep-order / append-order / upsert / remove+readd / empty-aux / TagBitset / dup-key last-wins / malformed-aux truncation / aux-offset-past-end clamp), and the zipper has_pg → adds_no_pg branch.

Reading order

1. crates/fgumi-raw-bam/src/tags.rs — rebuild_with, TagKeySet, TagBitset, and the contract/oracle tests.
2. src/lib/commands/zipper.rs — merge_raw_with (the reorder, the empty-set fast paths, and the transform post-pass).
3. crates/fgumi-sam/src/alignment_tags.rs — the NM/UQ/MD strip.

Follow-ups (deferred)

Re-profile fgumi zipper at the realistic 1–2 threads to size the tag-merge slice's real weight. The zipper profile also flagged a separate, larger win — the SAM→BAM RecordBuf re-encode of the mapped input — which is its own PR.

Output risk: yes—zipper tag fields and NM/UQ/MD cleanup can change; contract, oracle, and byte-identical tests pin the intended output. Unsafe: none. CLAUDE.md allowlist: unchanged. Memory bounds, queue capacity, and thread/backpressure policy: none.

• Added RawTagsEditor::rebuild_with for single-pass tag rebuilding.
• Added public TagKeySet and TagBitset APIs.
• Updated zipper tag merging and alignment-tag cleanup.
• Preserved ordering, upsert semantics, mapped PG behavior, and strand transforms.
• Added tests for ordering, duplicates, malformed data, and filtering.

[coderabbitai]

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: Essentials

Run ID: 778a6f94-21fb-4f51-9cc6-27fa3499c67a

📥 Commits

Reviewing files that changed from the base of the PR and between a9814c2 and 383b456.

📒 Files selected for processing (5)

• crates/fgumi-raw-bam/src/lib.rs
• crates/fgumi-raw-bam/src/tags.rs
• crates/fgumi-sam/src/alignment_tags.rs
• crates/xtask/src/check_tag_literals.rs
• src/lib/commands/zipper.rs

Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

---

Walkthrough

The PR adds raw-byte tag membership and single-pass auxiliary-field rebuilding. Alignment cleanup and zipper tag transfer use the new API, with PG precedence, deduplication, malformed-data handling, and expanded tests.

Changes

Raw tag rebuild and integration

Layer / File(s)	Summary
Tag membership and rebuild APIs crates/fgumi-raw-bam/src/tags.rs, crates/fgumi-raw-bam/src/lib.rs, crates/xtask/src/check_tag_literals.rs	Adds TagKeySet, TagBitset, and RawTagsEditor::rebuild_with. The implementation preserves survivors, appends additions, applies last-write-wins behavior, and handles malformed or truncated data. Tests cover these cases and the public exports include the new types.
Alignment tag cleanup crates/fgumi-sam/src/alignment_tags.rs	Removes NM, UQ, and MD with one rebuild operation for unmapped and invalid mapped records.
Zipper tag transfer and filtering src/lib/commands/zipper.rs	Uses cached raw-byte membership and single-pass rebuilding for tag removal and copying. Existing mapped PG tags take precedence, copied tags are deduplicated, transformations run once, and QC flag transfer uses a shared helper. Tests cover these behaviors and tag-name filtering.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Refactor

Sequence Diagram(s)

sequenceDiagram
  participant ZipperTags
  participant UnmappedRecord
  participant RawTagsEditor
  participant MappedRecord
  ZipperTags->>UnmappedRecord: Filter transferable tags
  ZipperTags->>RawTagsEditor: Rebuild mapped auxiliary fields
  RawTagsEditor->>MappedRecord: Preserve PG and append deduplicated tags
  ZipperTags->>MappedRecord: Synchronize QC flag

Loading

Merge Risk: ⚪ Minimal · up to 383b4

No confirmed merge-blocking issue remains in this change.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title uses a valid Conventional Commit type and no trailing period, but its description is a noun phrase rather than lowercase imperative wording.	Use an imperative description, such as: "perf: rebuild auxiliary tags in one pass for zipper merge and NM/UQ/MD stripping".

✅ Passed checks (2 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.

• Fix all pre-merge checks with AI

---

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

[nh13]

@coderabbitai pause

[coderabbitai]

✅ Action performed

Reviews paused.

[codecov]

Codecov Report

❌ Patch coverage is 98.07692% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.04%. Comparing base (a9814c2) to head (383b456).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/fgumi-raw-bam/src/tags.rs	98.68%	3 Missing ⚠️
src/lib/commands/zipper.rs	96.25%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #962      +/-   ##
==========================================
+ Coverage   96.03%   96.04%   +0.01%     
==========================================
  Files         291      291              
  Lines      143872   144087     +215     
==========================================
+ Hits       138167   138393     +226     
+ Misses       5705     5694      -11     

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