Correct combined and dynamic pileup count slots

[SuhasSrinivasan]

Fixes #655.

Summary

• Correct combined-C accounting so every selected C modification and every filtered call is represented.
• Make dynamic and high-depth count slots canonical-base-aware, align reverse ancillary counts to the combined motif anchor, and normalize exact duplicate selections.
• Add exact bedMethyl regressions for combined classes, compact slots, same-code/different-base selections, reverse ancillary categories, duplicate selections, and the debug CpG overlap path.

Severity

Severity: High — scientific correctness

Rationale: The affected commands can exit successfully while omitting passing modification classes, losing selected bases, splitting one requested category into contradictory rows, or undercounting failure and ancillary categories. These errors change reported coverage and modification percentages.

Root cause

The related failures arise from five narrow assumptions in the combined/dynamic count path:

1. The optimized combined-C matrix treats only m and h as modified and assigns the filtered-C counter to itself.
2. Compact combined slots use raw canonical-base enum values even though the allocated vector contains only selected bases; explicit slots search by raw modification code without the canonical base.
3. Reverse canonical/modified/depth paths apply the motif offset, while deletion, filtered, difference, and no-call paths use the unshifted coordinate.
4. Exact duplicate --modified-bases values reach preset and writer construction as separate categories.
5. A debug-only assertion rejects overlapping internal masks even though the following selection logic already defines deterministic first-hit behavior.

Implementation

• Treat every modified C call admitted by the combined-C preset as combined modified coverage, and increment N_fail with saturation.
• Remove only the invalid overlapping-mask debug assertion; retain existing first-hit routing.
• Build compact combined slots from the selected canonical bases, key explicit modification slots by (canonical base, modification code), and use explicit canonical/other-mod offsets instead of enum arithmetic.
• Use one dynamic_anchor_rpos helper for all dynamic call categories and max-depth lookup.
• Stably retain the first exact (canonical base, modification code) selection before preset and writer construction, warning for ignored duplicates while preserving the same raw code on different canonical bases.

The seven commits keep each root cause and its regression independently reviewable.

Preserved behavior

• Unique selections, supported modification syntax, CLI schema, and bedMethyl columns are unchanged.
• A:m and C:m remain distinct even though they share a raw modification code.
• Release-mode first-hit motif selection remains unchanged.
• Existing specialized CpG output and all unaffected pileup integration fixtures retain their expected bytes.

Non-goals

• Sampling, threshold estimation, ambiguous query bases, motif/cardinality limits, and general worker-error propagation are unchanged.
• This does not redesign the count-matrix representation or attempt a performance refactor.
• No new modified-base syntax or bedMethyl schema is introduced.

Behavior before and after

Case	Before	After	Expected oracle
Combined C with canonical + m + h + f + ChEBI 19228 + one filtered call	N_valid=3, N_mod=2, N_canonical=1, N_fail=0	N_valid=5, N_mod=4, N_canonical=1, N_fail=1	Every passing call and the filtered call are conserved exactly
C-only --combine-mods, normal versus high-depth	One row versus empty output	Identical one-row output	Byte identity below max-depth saturation
A:m C:m on one modified and one canonical ACT read	Correct A row; C split into 100% and 0% rows	One 50% row for A and one for C	One category per (base, code), each with coverage 2
A/C/T combined selection	T omitted	A, C, and T each emitted	All selected canonical bases retain a compact slot
Reverse GATC 1 --combine-strands ancillary categories	Delete/fail/diff/no-call all zero at the owned row	Each category is one	Every category shares the canonical/modified anchor
Duplicate C:m C:m in dynamic/high-depth mode	Two same-key rows	Byte-identical to unique C:m	Exact duplicate selection is idempotent
Debug CpG combined-C command	Panic	CpG-only output	Debug/release site-set parity

Testing

Test environment

• Revision tested: b060d18d35adc761f318e34353ce0b7f9bf00adb
• Tree tested and worktree state: 72c6ee8d08e6212dd1983c03d8a437772d9b4678; clean
• Parent revision: 5cecc3fb3a9336068d9e3c68d5c08d678153dd2c
• Toolchain: rustc 1.90.0; cargo 1.90.0
• Platform: macOS 26.6, arm64
• Reference binary: installed modkit 0.6.4
• External tools: samtools 1.23.1 / HTSlib 1.23.1
• Dependency resolution identity: ignored test-only Cargo.lock SHA-256 49c08c4c51b6f4320726551146d971fa9ef2183d40c3f6c631b2005965e242c0; one resolved hts-sys 2.2.0; the lockfile is not in this diff

Test layer	Exact command, fixture, or matrix	Result and evidence
Core: parent-red regression	The exact synthetic commands in #655 on installed 0.6.4, plus each regression on its immediate parent/mutation	Reproduced combined-C 3/2/1/0 instead of 5/4/1/1; empty C high-depth output; split same-code C rows; omitted reverse ancillary categories; duplicate dynamic/high-depth rows; and the debug overlap panic
Core: focused regression	cargo test -p mod_kit pileup::pileup_processor::tests -- --test-threads=1	4 passed, 0 failed, 0 ignored
Core: affected CLI/integration tests	cargo test -p modkit --test test_pileup -- --test-threads=1	22 passed, 10 declared ignored, 0 failed
Core: applicable full workspace gate	cargo test --workspace --all-targets -- --test-threads=1 in an isolated worktree/target	189 active tests passed, 14 declared ignored, 0 failed at the exact revision/tree
Installed-versus-fixed comparison	Three inline SAM fixtures from #655; installed modkit versus target/debug/modkit	Combined-C corrected from SHA-256 8647e2cda884d195ec63dcb0939050d11f0f25353f05f2c713db6b8b870897c5 to 86bfdef58f08f83ccaae3e462afa2d7281a7f46de6f9f22156f86425056b4dd8; C high-depth corrected from empty SHA-256 e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 to normal-output SHA-256 9e7c314f6914d756c77fb5a179394649427252c12a75656418a3574ba09592a8; reverse ancillary output corrected from e924d95e2dcc26dc2baca6e212c2eceb68df515e148c21570d296bc257c870f4 to 502c15d03c4b58806ae716cb33213eeddd8bffa050f0dbc2053e2381dd50ac86
Thread/interval/order matrix	Normal/high-depth at threads 1/2/3/8 for compact and explicit slots; normal/high-depth at threads 1/2 for reverse anchoring; interval size 1 or complete four-base motif interval	Exact expected bytes in every matrix cell
Format/strand/boundary matrix	C-only and A/C/T combined bases; A:m C:m; forward and reverse strands; phased ancillary unit partitions; non-CpG GATC 1; CpG overlap	All exact category, coordinate, strand, and site-set oracles pass
Duplicate-selection matrix	Unique versus duplicate selections in optimized, dynamic, and high-depth modes; same raw code on A versus C	Exact duplicates are byte-identical after one warning; different canonical bases remain distinct
Core: formatting/diff checks	rustfmt --edition 2021 --check modkit-core/src/pileup/pileup_processor.rs modkit-core/src/pileup/subcommand.rs modkit/tests/test_pileup.rs; git diff --check upstream/master...HEAD; clean-worktree check	Changed files and diff hygiene pass; worktree clean

Tests not performed

• The 1-GB direct-RNA regression was not rerun for this PR; the defects require exact synthetic category oracles, and the full workspace/affected integration gates passed.
• Performance, RSS, and I/O benchmarks were not run because this is a correctness-only change and does not alter the parallel scheduling or I/O architecture.
• cargo clippy was not run.
• cargo fmt --all -- --check reports unrelated pre-existing formatting drift in modkit/tests/test_bedmethyl_util.rs, modkit-core/src/adjust.rs, and modkit-core/src/entropy/mod.rs; all three changed files pass direct rustfmt --check.

Scientific validation

• Population/eligibility invariant: the same alignments and modification calls are eligible; only their destination count slots and duplicate option normalization change.
• Count/category conservation invariant: valid coverage equals canonical + selected modified + other modified; filtered observations enter N_fail once and remain outside valid coverage.
• Coordinate/strand/interval invariant: every reverse ancillary category uses the same combined motif anchor as canonical and modified calls.
• Determinism invariant: normal/high-depth and 1/2/3/8-thread results are exact-byte stable below saturation; duplicate selections are idempotent.
• Independent oracle or specialist review: exact fixtures and source seams were independently reviewed for biological counting, Rust invariants, and CPU/thread behavior; no blocking scientific, correctness, or performance concern remained.

Output and compatibility

• User-visible change: affected successful commands now emit corrected counts; ignored exact duplicate selections produce a warning.
• Expected output differences: only the incorrect cases listed above change.
• Byte-identical controls: unaffected pileup integrations, specialized CpG output, unique selections, and normal/high-depth controls pass exact comparisons.
• CLI/API/schema compatibility: unchanged.
• Partial-output or failure semantics: unchanged by this PR; the debug-only panic is removed for the accepted CpG command.

Reviewer guide

1. Review the four focused count-matrix tests at the end of modkit-core/src/pileup/pileup_processor.rs and their exact conservation oracles.
2. Review the combined-C increment changes, compact/base-code slot helpers, and dynamic_anchor_rpos call sites in the same file.
3. Review selection normalization in modkit-core/src/pileup/subcommand.rs and the five new CLI regressions in modkit/tests/test_pileup.rs.
4. Rerun these small canaries: cargo test -p mod_kit pileup::pileup_processor::tests -- --test-threads=1 and cargo test -p modkit --test test_pileup test_pileup_explicit_same_code_slots_are_keyed_by_base -- --exact.
5. Review the seven commits independently; each root cause remains isolated for bisection or reversion.

Checklist

• The issue contains reproducible observed and expected behavior.
• The change is limited to the linked issue’s approved scope.
• The regression is demonstrably red on the exact parent revision.
• All tests actually performed are listed above with their results.
• Unrun or inapplicable tests are disclosed.
• Scientific counts/statistics and output compatibility are explicitly checked.
• Formatting and diff-hygiene checks pass, or unrelated findings are documented.
• No generated data, private sample identifiers, or unrelated changes are included.