Correct repair matching, ordering, and atomic output - #711
Open
SuhasSrinivasan wants to merge 4 commits into
Open
Correct repair matching, ordering, and atomic output#711SuhasSrinivasan wants to merge 4 commits into
SuhasSrinivasan wants to merge 4 commits into
Conversation
This was referenced Aug 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review status: Author-reviewed and ready for ONT review.
Summary
This PR contains the complete repair-specific correctness package. It fixes three related failure modes in one reviewable branch:
The changes are consolidated because they all modify the same repair record-matching, scheduling, and output lifecycle. No pileup, extract, entropy, DMR, or probability code is included.
Severity
High — scientific output integrity and reliability.
The affected paths can silently place modification probabilities at the wrong acceptor positions, omit repairable records, retain a query-name-sorted header while reordering records, or report success after an incomplete output.
Bugs and fixes
ACACinACACACwas treated as unique because non-overlapping matching missed the second start at offset 2read1,read2,read3plus donorread1,read3could emit onlyread1read3is retainedSO:querynameheaderUnique forward/reverse repair and MM/ML/MN projection semantics are unchanged.
Implementation
memchr::memmem::Finderperforms at most two bounded searches so overlapping placements are detected without allocating all matches.0666 & umasksemantics; replacing an existing regular file preserves its permissions. Symlinks and nonregular destinations are rejected before staging.Testing
Tested exact revision:
23709cf3aff0da3aeec6beb02498629e0920aedfTree:
c5658ba199e533083f9bae4195d73772583820a8Platform/toolchain: macOS arm64; rustc/cargo 1.90.0;
rust-htslib0.46.0 andhts-sys2.2.0.ACACAC/ACACplacement; this branch rejects it.read3; this branch emitsread1,read3in acceptor order and counts one rejected record.52b8a3a227b08d0cd98d79c826421199971f5c2e2feead0e3d5c6441e25dc813. The result also matches installed modkit 0.6.4 at one worker.0404, live/dangling symlink, FIFO, and directory.git diff --check, targetedrustfmt --check, clean-worktree, ancestry, and Cargo.lock checks passed.The exact combined head was independently reviewed for Rust ownership/cancellation, matching, diagnostics, output lifecycle, and test preservation. No blocker remained.
Output compatibility
Reviewer guide
Findersearches inrepair_tags.rs.ordered_scheduler.rsfor bounded ordering, cancellation, first-error preservation, and joins.cargo test -p mod_kit repair_tags::tests -- --test-threads=1,cargo test -p mod_kit ordered_scheduler::tests -- --test-threads=1, andcargo test -p modkit --test test_repair -- --test-threads=1as focused canaries.Non-goals
fsync, or ACL/xattr/owner/group preservation.