From e2fc88fe75a842f7a5da9aa71379df557f69cb46 Mon Sep 17 00:00:00 2001 From: Nils Homer Date: Wed, 10 Jun 2026 22:16:33 -0700 Subject: [PATCH] fix(correct): rejects BAM carries the input header verbatim (#332 contract) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `fgumi correct --rejects` stamped the rejects BAM with header_as_unsorted(self.header), stripping the input's @HD GO/SS sort fields and forcing SO:unsorted. Per the PR #332 rejects contract, rejects are raw-input records (input order, already via correct's fan-out branch) and must carry the INPUT header verbatim. For correct, self.header IS the input header (correct preserves input order and does not swap in a consensus header), so use it as-is for the rejects writer. Fixes test_bgzf_eof::{test_correct_rejects_has_bgzf_eof, test_correct_single_threaded_rejects_has_bgzf_eof}. The simplex/duplex/codec rejects paths still use header_as_unsorted via their mutex side-channel; correcting those requires the full fan-out migration (input header + input order together, since a header-only change would mislabel the mutex-order records) — tracked separately. --- src/lib/pipeline/chains/builder.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/lib/pipeline/chains/builder.rs b/src/lib/pipeline/chains/builder.rs index bdb718013..1f186842d 100644 --- a/src/lib/pipeline/chains/builder.rs +++ b/src/lib/pipeline/chains/builder.rs @@ -1209,9 +1209,11 @@ impl<'a> ChainBuilder<'a> { /// presence + numeric ranges). Loads UMI sequences and encodes them into /// [`EncodedUmiSet`] up front. /// - /// The `rejects_header` is the primary output header stamped as `SO:unsorted` - /// via [`header_as_unsorted`] — matching the Phase 2 behaviour in - /// `build_correct_chain`. + /// The `rejects_header` is the **input header verbatim** (PR #332 contract: + /// rejects are raw-input records, so they carry the input's `@HD` sort + /// fields + RG/PG — not the consensus/output header). For `correct`, + /// `self.header` is the input header (correct preserves input order and does + /// not replace `self.header` with a consensus header), so it is used as-is. /// /// Registers a [`CorrectFinalizeHook`] for: per-thread accumulator reduce → /// `finalize_metrics` → summary + warn banners → `--min-corrected` ratio @@ -1226,7 +1228,6 @@ impl<'a> ChainBuilder<'a> { /// implemented). /// /// [`EncodedUmiSet`]: crate::commands::correct::EncodedUmiSet - /// [`header_as_unsorted`]: crate::sam::header_as_unsorted /// [`DecompressedBlock`]: crate::pipeline::steps::types::DecompressedBlock /// [`CorrectFinalizeHook`]: crate::pipeline::chains::commands::correct::CorrectFinalizeHook #[allow(clippy::too_many_lines)] @@ -1241,7 +1242,7 @@ impl<'a> ChainBuilder<'a> { }; use crate::pipeline::steps::group::queryname::GroupByQueryname; use crate::pipeline::steps::serialize::SerializeBamRecords; - use crate::sam::{SamTag, header_as_unsorted}; + use crate::sam::SamTag; use log::info; // `Intermediate` is used when correct feeds into align-and-merge @@ -1335,7 +1336,9 @@ impl<'a> ChainBuilder<'a> { let rejects_path = correct_opts.rejects_path.as_ref().ok_or_else(|| { anyhow!("rejects_path unexpectedly None (build_for dispatch bug)") })?; - let rejects_header = header_as_unsorted(&self.header); + // PR #332 contract: rejects carry the INPUT header verbatim (raw-input + // records, input order). For correct, `self.header` IS the input header. + let rejects_header = self.header.clone(); let rejects_write = WriteBgzfFile::new(rejects_path, &rejects_header, self.tuning.compression_level) .map_err(|e| anyhow!("WriteBgzfFile (rejects): {e}"))?;