Skip to content

Propagate text and DMR output failures - #697

Open
SuhasSrinivasan wants to merge 7 commits into
nanoporetech:masterfrom
SuhasSrinivasan:codex/propagate-text-dmr-output-failures
Open

Propagate text and DMR output failures#697
SuhasSrinivasan wants to merge 7 commits into
nanoporetech:masterfrom
SuhasSrinivasan:codex/propagate-text-dmr-output-failures

Conversation

@SuhasSrinivasan

Copy link
Copy Markdown
Contributor

Fixes #669.

Summary

  • Replace partial-write-prone text output calls with complete, fallible writes.
  • Propagate ordinary buffered-writer flush failures and retain the first DMR processing/output error while still joining owned stages and attempting independent flushes.
  • Add deterministic short-write, flush-failure, DMR segmenter, final-chunk, and thread-lifecycle regressions.

Severity

Severity: Medium — output integrity and reliability

Rationale: A legal short write or swallowed late failure can leave truncated or incomplete output while the command reports success. Successful scientific bytes are unchanged, but a zero exit status previously did not guarantee complete output.

Root cause

Several text writers used write where write_all was required, and some command paths dropped buffered flush, worker-join, or DMR segmenter errors. Error handling could return early before completing safe cleanup or flushing independent outputs.

Implementation

  • Use complete writes in shared text/bedMethyl writers and propagate ordinary flush errors.
  • Make DMR receiver/finalization paths retain their first processing error, finish or clean pending state safely, join owned stages, and attempt every independently flushable output.
  • Add narrow failing-writer seams rather than changing successful serialization.

Preserved behavior

  • Successful text, bedMethyl, extract, pileup, and DMR bytes remain unchanged.
  • The first substantive error remains the returned error; cleanup does not mask it.

Non-goals

  • No migration or vendoring of gzp::ParCompress consuming-finalization behavior.
  • No BAM close redesign and no general cancellation-policy redesign.

Behavior before and after

Case Before After Oracle
Legal short writer Row could be truncated Entire row written or an error returned Complete bytes or nonzero result
Buffered flush failure Error could be dropped Contextual error returned No false success
DMR add/final-chunk failure Pending segment/error could be lost First error retained while safe finalization is attempted Deterministic failure
Healthy output Existing bytes Unchanged Exact byte identity

Testing

Environment: macOS 26.6 arm64; rustc/cargo 1.90.0; installed modkit 0.6.4 reference; ignored test-only Cargo.lock SHA-256 78876ab4a98da30caad167744d1c8a0875c27edad7b0b3ad5f8a1891f78604ea resolving hts-sys 2.2.0. Cargo.lock is not in the diff.

  • Revision/tree: 096b046 / 7134426b9fddda1ff7c75eb61132912651f57e35; clean tracked worktree.
  • Parent-red fault injection reproduced partial writes and swallowed flush/segmenter/finalization errors.
  • cargo test --offline --locked -p mod_kit writer -- --test-threads=1: 9 passed, 0 failed.
  • cargo test --offline --locked -p mod_kit dmr:: -- --test-threads=1: 28 passed, 0 failed.
  • cargo test --offline --locked -p modkit --test test_dmr -- --test-threads=1: 2 passed, 0 failed.
  • Recorded lifecycle gates: 14/14; output-finalization 7/7; segmenter-error 7/7; exact DMR regression 1/1.
  • Exact-head full workspace/all-targets gate: 202 active tests passed, 14 declared ignored, 0 failed.
  • 898-MB direct-RNA stress BAM: threads 1 and 8 each emitted 131,900 rows, byte-identical to installed 0.6.4, SHA-256 2c98dc0193764a410a8d84b1227733ec101f47db5541ec7c313e7d07e1a447c8.
  • git diff --check upstream/master...HEAD passed; the worktree remained clean.
  • Repository-wide stable cargo fmt --all -- --check reports unchanged upstream formatting plus nightly-only settings; no unrelated formatting rewrite was made.
  • Test setup note: an initial writers::tests filter selected zero tests. The corrected writer filter selected and passed all nine writer tests.

Tests not performed

  • cargo clippy was not run.
  • Failure injection for gzp consuming finalization and BAM close was intentionally not performed because those paths are outside this PR.

Scientific validation

  • Successful output is byte-identical, including the direct-RNA stress comparison.
  • Every independently flushable output is attempted after a failure and the first error is preserved.
  • DMR segment state is neither silently dropped nor emitted twice.
  • Thread 1/8 output identity is retained.

Output and compatibility

  • User-visible change: scoped writer failures now produce a nonzero error instead of possible truncated success.
  • CLI/schema compatibility and successful bytes are unchanged.
  • The upstream gzp limitation remains documented separately and is not implied to be fixed here.

Reviewer guide

  1. Review ShortWriter/failing-flush tests in modkit-core/src/writers.rs.
  2. Review the DMR receiver error-retention/finalization paths.
  3. Rerun the three focused commands above.

Checklist

  • The issue contains reproducible observed and expected behavior.
  • The change is limited to modkit-owned writer and DMR lifecycle code.
  • Parent-red and fix-green evidence is recorded.
  • All material tests and setup notes are listed.
  • Successful scientific bytes and failure semantics are checked.
  • Diff hygiene passed; unrelated format findings are disclosed.
  • No private data, generated lockfile, or unrelated change is included.

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.

Shared text and DMR output failures can be silently discarded

1 participant