`pileup` panics above 16 MM-code slots and silently drops motifs beyond eight

[SuhasSrinivasan]

pileup panics above 16 MM-code slots and silently drops motifs beyond eight

Summary

Two accepted-input cardinalities can exceed fixed internal storage in pileup:

• The optimized MM adapter stores modification-code state in fixed arrays and asserts when a valid MM tag expands to more than 16 code slots.
• Multiple-motif membership is packed into a u8, but the CLI accepts more than eight motifs. Release builds silently omit the ninth and later requested labels; debug behavior can overflow/panic.

The proposed package keeps the two repairs as separate commits: dynamically support MM-code state beyond 16, and explicitly reject motif counts beyond the current eight-bit representation before output creation.

Severity

Severity: High — reliability and scientific completeness

Rationale: The 17-code case visibly aborts on valid tags after check-tags accepts them. The ninth-motif case is more severe scientifically because the command exits successfully with plausible but incomplete output. Both boundaries are option/data specific but affect accepted inputs without a preflight diagnostic.

User and scientific impact

• Affected result or workflow: optimized pileup on large MM code sets; generic multi-motif pileup with more than eight effective motifs.
• Direction of error: panic/empty partial output, or silent omission of requested motif labels and their rows.
• Likely exposure: uncommon 17-code tags; option-specific multi-motif analyses.
• Detectability or workaround: inspect tag cardinality and keep requests within 16/8 respectively. For motifs, a successful exit does not reveal that labels were silently omitted unless the requested and observed label sets are compared.

Affected versions and environment

• Affected release: reproduced with modkit 0.6.4.
• Development revision: 5cecc3fb3a9336068d9e3c68d5c08d678153dd2c.
• Operating system and architecture used for verification: macOS on Apple Silicon.
• Input format: valid modBAM; indexed reference FASTA for pileup.

Steps to reproduce

1. Valid 17-code MM tag

Create a one-base reference and one valid 17-code record. Only code q has a nonzero ML value.

printf '>chr1\nC\n' > ref.fa
samtools faidx ref.fa

printf '@HD\tVN:1.6\tSO:coordinate\n@SQ\tSN:chr1\tLN:1\nseventeen-codes\t0\tchr1\t1\t60\t1M\t*\t0\t0\tC\t?\tMM:Z:C+abcdefghijklmnopq?,0;\tML:B:C,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,200\tMN:i:1\tNM:i:0\n' > input.sam
samtools view -b -o input.bam input.sam
samtools index input.bam

modkit modbam check-tags input.bam --head 1 --suppress-progress

modkit pileup input.bam output.bed \
  --ref ref.fa \
  --modified-bases C:q \
  --no-filtering \
  --threads 1 \
  --io-threads 1 \
  --suppress-progress

Observed behavior

check-tags exits zero, reports one passing record, and lists all 17 codes. The optimized pileup then exits 101 after an internal assertion:

assertion failed: n_mods_so_far < SIZE

On the reproduced run it left a zero-byte output file.

Control or independent oracle

The MM group is valid under the tag parser. A dynamically sized adapter must retain 17 code states and emit the selected q call with raw ML 200. Boundary controls include 16/17 codes in one group, 16/17 one-code groups, one code at 17 positions, and reverse two-position/17-code ML-stride traversal.

2. Ninth requested motif

From a modkit checkout containing the repository resources, first run exactly eight overlapping motifs:

modkit pileup \
  tests/resources/CG_5mC_20230207_1700_6A_PAG66026_3c0abf27_oligo_741_adapters_modcalls_0th_sort_10_reads.bam \
  eight.bed \
  --no-filtering \
  --ref tests/resources/CGI_ladder_3.6kb_ref.fa \
  --region oligo_741_adapters:22-62 \
  --modified-bases 5mC \
  --threads 1 \
  --io-threads 1 \
  --suppress-progress \
  --motif CGG 0 \
  --motif CGGG 0 \
  --motif TCG 1 \
  --motif CTCG 2 \
  --motif GCTCG 3 \
  --motif TGCTCG 4 \
  --motif TTGCTCG 5 \
  --motif ATTGCTCG 6

Add --cpg to the same command, change the output to nine.bed, and compare the distinct labels:

cut -f4 eight.bed | sort -u
cut -f4 nine.bed | sort -u

Observed behavior

The eight-motif command exits zero with eight distinct labels. The nine-motif command also exits zero, but still contains only the original eight labels: the auto-added m,CG,0 label and its rows are silently absent.

Control or independent oracle

Every accepted requested motif must be represented. Until the membership mask is widened, a ninth effective motif should fail before creating/truncating output with a diagnostic stating the supported limit and received count. Validation must run both before and after automatic CpG/base-derived motif additions.

Expected behavior

• Valid 17-code MM layouts do not panic. States beyond the inline capacity remain addressable with correct ML stride and forward/reverse progression.
• Ordinary layouts through 16 code slots remain allocation-free and scientifically unchanged.
• Eight effective motifs remain accepted.
• Nine or more effective motifs fail before output creation with a clear at most 8 motifs / received 9 diagnostic, including auto-added motif cases.
• Supported-input output schema, order, and counts remain unchanged.

Root-cause evidence

• modkit-core/src/pileup/base_mods_adapter.rs: several parallel arrays encode code state with a fixed capacity and assert while parsing slot 17.
• modkit-core/src/pileup/pileup_processor.rs: motif matches are represented by bits in one u8, so only eight labels can be retained.
• modkit-core/src/pileup/subcommand.rs: the affected parent accepts more than eight motifs without validating the mask capacity.
• Parent regressions reproduce both valid 17-slot panics and eight-of-nine silent motif output.

Proposed fix scope

• Replace fixed parallel adapter state with one stack-first SmallVec<ModCodeState> retaining inline capacity 16 and spilling only beyond it.
• Derive the motif limit from u8::BITS and validate the explicit and post-auto-add effective motif counts before opening output.
• Keep dynamic code support and motif-capacity validation as two separate commits in one cardinality-focused PR.

Non-goals

• No dynamic motif mask or support for more than eight motifs in this repair.
• No entropy or other command-family cardinality changes.
• No SAM-tag grammar/policy change, ambiguity handling, independent-MM-group progression, output schema/order change within supported limits, scheduling, I/O, or broad performance refactor.

Acceptance criteria

• Valid 16/17 code-slot layouts, separate groups, one-code/many-position, and reverse ML-stride controls produce exact calls without panic.
• The selected 17th q call is emitted with raw ML 200.
• Exact eight-motif requests succeed and preserve all eight labels.
• Explicit or automatically produced ninth-motif requests fail before creating or changing output with the exact capacity diagnostic.
• The repaired adapter is composed after the independent-MM-group fix with a dynamically sized readiness mask.
• Focused adapter/motif tests, the applicable full workspace suite, and a representative direct-RNA byte/performance guard pass after final composition.

Reproduction artifacts

Artifact	Size	SHA-256	Notes
input.sam	169 bytes	1cfe057dad78b6173de4ebc7ab0490c6e11259a21a91960a342575388dc0cc50	Inline 17-code SAM above.
input.bam	282 bytes	ed4eec1c74748de6f16cc5c91d7d2936e4532429d15386346de51345c9c8b390	samtools 1.23.1 conversion.
input.bam.bai	96 bytes	efa1cfcba94d5bae162ef2186c71c259c89c61143337a37e6d07b18cde893330	samtools 1.23.1 index.
tests/resources/CG_5mC_..._10_reads.bam	5,496 bytes	f32816ce26c0e165cce243d6e614f18c673939d0165b1cd152547eb2ffdff765	Existing motif fixture; full filename appears in the command.
tests/resources/CGI_ladder_3.6kb_ref.fa	9,606 bytes	7c02f1e6dc3a3dae186ae47696c822efafe849b8c8526bc131237bf7c3a64570	Existing reference fixture.

Related work

• Related issues/PRs: issue pileup can panic or lose downstream calls on valid ambiguous query bases #668/PR Allow N #597 cover ambiguity handling; issue pileup can omit or split counts for valid combined and explicit modification selections #655/PR Correct combined and dynamic pileup count slots #656 cover separate pileup slot/count mapping.
• Proposed PR: to be linked only after the independent-MM-group fix and overlapping pileup work land, these two commits are rebuilt on current upstream, and the combined branch passes fresh workspace/direct-RNA gates.
• Deferred policy question or upstream dependency: more-than-eight dynamic motif support is intentionally deferred; PR Pin hts-sys to bindings compatible with rust-htslib 0.46 #666 affects the publication rebase/build gate.