Skip to content

fix: guard against missing MC tag in allele frequency calculation#58

Merged
nh13 merged 2 commits intomainfrom
fix/mate-end-unsafe-get
Mar 2, 2026
Merged

fix: guard against missing MC tag in allele frequency calculation#58
nh13 merged 2 commits intomainfrom
fix/mate-end-unsafe-get

Conversation

@nh13
Copy link
Member

@nh13 nh13 commented Feb 20, 2026

Summary

  • Add rec.mateEnd.isDefined check to checkAsPair in AggregateSvPileup.numOverlappingTemplates
  • Prevents NoSuchElementException when BAM files lack the MC (mate CIGAR) tag, which is needed to derive mateEnd
  • Records without the MC tag now fall through to single-read overlap checking instead of crashing

Test plan

  • Existing tests pass (./mill tools.test)
  • Verify with a BAM file that lacks MC tags (e.g. older BAMs or certain aligner outputs)

@nh13 nh13 requested review from clintval and tfenne as code owners February 20, 2026 21:49
@nh13 nh13 force-pushed the fix/mate-end-unsafe-get branch from 8b801d3 to edb5140 Compare February 21, 2026 01:55
@coderabbitai
Copy link

coderabbitai bot commented Feb 21, 2026

Warning

Rate limit exceeded

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between ca7b619 and 8187d90.

📒 Files selected for processing (2)
  • docs/tools/index.md
  • src/main/scala/com/fulcrumgenomics/sv/tools/AggregateSvPileup.scala
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/mate-end-unsafe-get

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@nh13 nh13 requested a review from clintval February 21, 2026 05:58
@nh13 nh13 assigned clintval and unassigned nh13 Feb 21, 2026
@nh13 nh13 force-pushed the fix/mate-end-unsafe-get branch from d63ed81 to a4edb83 Compare February 21, 2026 06:20
@clintval clintval assigned nh13 and unassigned clintval Feb 26, 2026
…uency calculation

The mateEnd.get call in numOverlappingTemplates would throw a bare
NoSuchElementException when the BAM file lacks MC (mate CIGAR) tags.

Replace with getOrElse that raises an IllegalStateException with a
clear message directing users to add MC tags (e.g. via samtools
fixmate). Keep checkAsPair unchanged since it does not access mateEnd.
@nh13 nh13 force-pushed the fix/mate-end-unsafe-get branch from 20cbb59 to 48b6570 Compare March 2, 2026 00:17
@nh13
Copy link
Member Author

nh13 commented Mar 2, 2026

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Mar 2, 2026

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

@nh13 nh13 merged commit 7ab12ae into main Mar 2, 2026
1 check passed
@nh13 nh13 deleted the fix/mate-end-unsafe-get branch March 2, 2026 03:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants