Allow N

[killercup]

We were testing with a BAM file where reads have Ns and modkit crashed on the unreachable! in modkit-core/src/pileup/base_mods_adapter.rs:77. I think this "fixes" this by ignoring this position. I also tried to come up with some other places this could be problematic and then also tried to have Claude write tests that exercise this.

[ArtRand]

Hello @killercup,

Out of curiosity, where are you getting sequencing reads with N bases?

Thanks for the contribution, I'll take a look when I start rolling up all the changes for v0.6.2. One thing of note, this code is part of the hottest most critical loop in pileup, so I'll have to make sure there isn't any performance regression. From a quick glance, it should be fine, but I'll need to check. If you have a samply trace from the released version and this one that would save me some time.

[killercup]

Thanks for the quick reply! We're working on https://bitbucket.org/bsblabludwig/rastair which works with TAPS reads and can write modBAM.

Good point about the performance, I can compare this for you. (Might have to wait until tomorrow though)

[ArtRand]

Yes I saw Rastair, looks like a nice project! No rush on the perf data. Thanks for the contribution.

[SuhasSrinivasan]

Thank you for fixing the original reverse-scanner N crash. I compared the current head of this PR (a6115966) with our overlapping work and found two remaining ordinary-IUPAC paths worth folding into this PR:

1. In both the optimized and generic workers, an ambiguous aligned base in the q < next modification path still abandons the complete record. A leading or internal N can therefore remove a valid downstream modified call and its coverage (and can leave only earlier partial counts).
2. In the generic worker's overrun path, query-base conversion is still unwrapped before the newly discovered pending modification is saved. An ambiguity between an earlier unaligned call and a later aligned call can therefore panic and lose the later call.

The minimal completion is to skip only the unclassifiable aligned observation and retain the pending state, saving newly advanced state before fallible conversion. Exact optimized/generic regressions are in 85d62ea:

• A leading-N fixture requires the downstream 5mC row at chr1:1-2 with modified count and valid coverage both equal to 1.
• A soft-clipped earlier call followed by an aligned ambiguity requires the same downstream call in both workers and must not panic.
• The adapter oracle in aa15346 verifies exact forward/reverse MM positions and ML-derived qualities when R/N/Y occur between canonical bases.

I am closing our overlapping PR #714 so this remains the single active PR for ordinary IUPAC handling; its branch and tests remain available as implementation evidence.

Performance context, with an important limitation: these numbers test the separate broader implementation in #714, not this PR. Against upstream 5cecc3f, release pileup on an 898,198,413-byte/1,632,831-read direct-RNA BAM used one warm-up and three interleaved runs per binary. Upstream wall-time median/mean was 6.87/6.8467 s; #714 was 6.75/6.7567 s. We treat the small difference as noise and claim only no measurable material regression for #714. All outputs were byte-identical: 60,959 rows, 9,989,028 bytes, SHA-256 a85202b69e79e708656e71858e4243cb1cc51e22a44bb9f65650a23dacb407fa. The sampled normal-data scan contained no non-ACGT query bases, and no samply trace was collected. A fair benchmark of #597 still requires applying it to the same upstream revision and rerunning the same interleaved protocol.