Validate bedMethyl merge inputs and preserve row identities

[SuhasSrinivasan]

Fixes #663.

Summary

• Validate every requested bedMethyl input before opening the merge output instead of silently removing failed readers.
• Preserve the complete column-4 name as part of merge identity and output.
• Count distinct requested inputs, not duplicate rows within one input, toward --min-samples.
• Add focused failure-preservation, index-format, byte-identity, motif/name, duplicate-row, and ordering regressions.

Severity

Severity: High — scientific correctness, sample membership, and output integrity

Rationale: The affected command can exit successfully after omitting a requested sample, combining biologically distinct motif rows, or falsely satisfying a sample-support filter. Its output remains syntactically valid, making the scientific error difficult to detect downstream.

Root cause

The merge reader list was built with filter_map, discarding any data/index open error after --min-samples all had already been resolved against the requested input count. The output was opened first, so even an eventual failure could overwrite an existing result. Separately, BedMethylLine retains only the modification-code component of column 4; merge keyed and serialized that reduced representation, and incremented sample support once per row.

Implementation

• Collect all HtsTabixHandler construction results with path context before creating the Rayon pool or output writer. This preserves requested cardinality and fails without touching output.
• Add a private merge-only wrapper around BedMethylLine that also retains the exact column-4 name. The generic parser and public schema remain unchanged.
• Include the retained name in the merge key and deterministic sort order.
• Maintain a per-reader set of contributed identities. All qualifying duplicate rows still add their eight numeric fields, but only the first matching row from that reader increments n_samples.
• Keep .csi-before-.tbi index discovery through the existing Tabix handler and avoid an O(total-input) pre-read.

Preserved behavior

• Valid inputs with bare modification-code names retain byte-identical output and existing count aggregation.
• All eight numeric bedMethyl count fields are still summed across every qualifying row.
• Existing row ordering remains coordinate, strand, modification code, with the complete name as the final deterministic tie-breaker.
• Passing the same path twice still represents two requested samples, matching the existing CLI contract.
• Valid .tbi and .csi indexes remain accepted.

Non-goals

• Late BGZF/decode failures after successful reader/index construction remain a separate pipeline-lifecycle issue.
• No writer/channel cancellation or compressed-output finalization redesign.
• No full stream pre-read, performance redesign, public API change, or output-schema change.
• No changes outside bedmethyl merge.

Behavior before and after

Case	Before	After	Expected oracle
m,CG,0 and m,DRACH,2 at one position in two inputs	One combined m row	Two motif-specific rows	Exact full column-4 identity retained
Two h,CG,0 rows in only one of two inputs with --min-samples all	h row falsely retained	h,CG,0 excluded	One sample contribution per requested input and identity
One valid plus one missing input with --min-samples all --force	Exit 0; missing input skipped; existing output overwritten	Exit 1; path identified; existing output preserved	All requested inputs validated before output creation
Existing ordinary valid merge fixture	Successful merge	Byte-identical successful merge	No compatibility change outside affected identities

Testing

Test environment

• Revision tested: ac7ae9da532e89a6f453b90bb980efb97955f599
• Tree tested and worktree state: 73dab255ceb39a7c1ec6b5531c0191ce352c18c6; 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: bgzip/tabix from HTSlib 1.23.1
• Dependency resolution identity: ignored test-only Cargo.lock SHA-256 49c08c4c51b6f4320726551146d971fa9ef2183d40c3f6c631b2005965e242c0; 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	Public two-file and missing-input commands from the linked issue on installed modkit 0.6.4/current-parent logic	Exit 0 with names collapsed to m/h; duplicate-only h retained; missing input silently skipped and sentinel overwritten
Core: focused identity regression	cargo test --offline --locked -p modkit --test test_bedmethyl_util test_bedmethyl_merge_preserves_full_name_and_counts_distinct_inputs -- --exact --test-threads=1	1 passed, 0 failed; exact bare, motif-bearing, and numeric-ChEBI names; duplicate support; reversed input/row order
Core: focused preflight regression	cargo test --offline --locked -p modkit --test test_bedmethyl_util test_bedmethyl_merge_rejects_one_bad_input_with_min_samples_all -- --exact --test-threads=1	1 passed, 0 failed; causal path and pre-existing output sentinel asserted
Unit/library tests	cargo test --offline --locked -p mod_kit bedmethyl -- --test-threads=1	6 passed, 0 failed; 92 filtered out
Core: affected CLI/integration tests	cargo test --offline --locked -p modkit --test test_bedmethyl_util -- --test-threads=1	10 passed, 0 failed, including missing data, absent/corrupt index, CSI, byte identity, coverage, and sample thresholds
Core: applicable full workspace gate	cargo test --offline --workspace --all-targets -- --test-threads=1 on the frozen exact head	Every active workspace/all-target test passed with 0 failures; core reported 95 passed and 3 declared ignored in that gate
Installed-versus-fixed comparison	Synthetic fixture from the linked issue, --threads 1 --io-threads 1 --min-samples all	Installed SHA-256 60ee4d996c0d5cb7d8218f77e383f58213567860268168f1245daf1a2888cb42; fixed exact two-row SHA-256 485e6f10ec492710ab2f95d3de9d8a5327bf823c57074fd6df795ea8bd9c3c22
Failure preservation	Valid input plus absent path, existing sentinel, --force --min-samples all	Installed exits 0 and overwrites sentinel; fixed exits 1, names absent.input.bed.gz, and preserves 15-byte sentinel with SHA-256 db5a4b107e2107da20a4e94e5aaf4f379752c06a1367e00ed279ae5a9c1ab729
Input/name/order matrix	Bare a, m,CG,0, m,DRACH,2, numeric 76792,CG,0; duplicate rows; reversed reader and row order	Exact expected bytes in both orders; one-input-only duplicate identity excluded under all
Core: formatting/diff checks	Direct rustfmt --edition 2021 --check on both changed Rust files; git diff --check upstream/master...HEAD; clean-worktree check	Passed; stable rustfmt emitted only warnings for nightly-only repository settings; worktree clean at the tested SHA

Tests not performed

• The 1-GB direct-RNA BAM slice was not run because this command consumes indexed bedMethyl, and the synthetic fixtures provide exact identity, count, index, and failure oracles.
• Performance, RSS, and I/O benchmarks were not run because the change is correctness-only and does not add a full stream pre-read.
• cargo clippy was not run.
• A late truncated-BGZF decode failure was not promoted into this PR; it is explicitly deferred as a separate pipeline-lifecycle issue.

Scientific validation

• Population/eligibility invariant: --min-samples counts the requested input files that contribute a qualifying matching identity, never the number of rows.
• Count/category conservation invariant: all eight numeric count fields are summed for every qualifying row; only the sample-support tally is de-duplicated per input/identity.
• Identity invariant: coordinate, strand, parsed modification code, and exact bedMethyl column-4 name jointly identify a merge row.
• Determinism invariant: exact output is unchanged under the tested reversal of input and within-input row order.
• Independent oracle: hand-summed two-input counts, exact expected bytes, failure exit/output-preservation checks, and independent specialist/supervisor code review all agree.

Output and compatibility

• User-visible change: motif-bearing and numeric-ChEBI column-4 names are retained; invalid requested inputs now fail loudly before output creation.
• Expected output differences: previously collapsed identities split into their correct rows; rows admitted only by duplicate same-file support are removed.
• Byte-identical controls: the existing ordinary valid merge fixture is explicitly asserted byte-for-byte.
• CLI/API/schema compatibility: unchanged; the retained-name wrapper is private to merge.
• Partial-output or failure semantics: data/index preflight failures are nonzero and leave existing output untouched. Later decode/writer lifecycle behavior is unchanged and deferred.

Reviewer guide

1. Review test_bedmethyl_merge_preserves_full_name_and_counts_distinct_inputs for the exact scientific count and identity oracle.
2. Review the private merge wrapper and key in modkit-core/src/bedmethyl_util/subcommands.rs.
3. Review input-reader collection placement to confirm it precedes output creation without pre-reading the full streams.
4. Review the four preflight regressions and valid CSI/byte-identity controls.
5. Rerun the two focused commands above, then the 10-test test_bedmethyl_util target.

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/release logic.
• 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.