From c236b4dca135ea6e4096853d9fcade09ee19b253 Mon Sep 17 00:00:00 2001 From: Benjamin Riley Date: Sun, 19 Jul 2026 21:58:38 +0000 Subject: [PATCH] feat: implement Reader::iter_chromatograms emitting a TIC chromatogram Wire the already-decoded Idx TIC data into a total ion current chromatogram (MS:1000235) via a SpectrumSource::iter_chromatograms override. One point per IdxRecord: time_sec from retention_time_min * 60.0, intensity from the record's tic (cps). No new raw-format decode work, only wiring existing fields into openmassspec_core::ChromatogramRecord, so TIC chromatograms now appear in the mzML OpenSXRaw produces. The per-spectrum SpectrumRecord.total_ion_current field is left None as before: that value must match sum(raw intensities) for the conformance suite's rel_close check, which is a per-spectrum constraint and does not apply to a separate chromatogram trace (assert_source_invariants never inspects iter_chromatograms). BPC and SRM/MRM are intentionally out of scope: a per-scan base peak is not decoded today, and no Q1/Q3 transition-level data is decoded anywhere in this reader. Both need net-new decode work and should be separate follow-up issues. Adds unit tests for the chromatogram wiring and a corpus-gated integration test. Refs Sigilweaver/OpenSXRaw#21 Claude-Session: https://claude.ai/code/session_01TLSifJpmaN7RvW3xi2uWgo --- CHANGELOG.md | 15 ++++ crates/opensxraw/src/reader.rs | 123 +++++++++++++++++++++++++- crates/opensxraw/tests/conformance.rs | 29 ++++++ 3 files changed, 166 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bb3ca14..7929a1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- `Reader::iter_chromatograms` (Sigilweaver/OpenSXRaw#21): emits a single + total ion current chromatogram (`MS:1000235`) built from the already-decoded + `idx_records` - one point per record, `time_sec` from `retention_time_min` + and `intensity` from the record's Idx `tic` (cps). No new raw-format decode + work is involved, only wiring existing fields into `ChromatogramRecord`, so + TIC chromatograms now appear in the mzML `` OpenSXRaw + produces. The per-spectrum `SpectrumRecord.total_ion_current` field stays + `None` as before (that value must match `sum(raw intensities)` for the + conformance suite's `rel_close` check, which does not apply to a separate + chromatogram trace). Basepeak (BPC) and SRM/MRM chromatograms are + intentionally left out - both require net-new decode work and should be + tracked as separate follow-up issues. (contributed by @Nabejo) + ## [0.2.1] - 2026-07-15 ### Fixed diff --git a/crates/opensxraw/src/reader.rs b/crates/opensxraw/src/reader.rs index b17d074..268160c 100644 --- a/crates/opensxraw/src/reader.rs +++ b/crates/opensxraw/src/reader.rs @@ -5,7 +5,8 @@ use std::path::{Path, PathBuf}; use cfb::CompoundFile; use openmassspec_core::{ - Analyzer, CvTerm, PrecursorInfo, RunMetadata, ScanMode, SpectrumRecord, SpectrumSource, + Analyzer, ChromatogramRecord, CvTerm, PrecursorInfo, RunMetadata, ScanMode, SpectrumRecord, + SpectrumSource, }; use crate::raw::calibration::Calibration; @@ -353,4 +354,124 @@ impl SpectrumSource for Reader { Box::new(iter) } + + fn iter_chromatograms<'a>(&'a mut self) -> Box + 'a> { + // Emit a single total-ion-current chromatogram built entirely from + // data already decoded into `idx_records`: one point per Idx record, + // `time_sec` from `retention_time_min * 60.0` and `intensity` from the + // record's `tic` field. No new raw-format decode work is involved, + // only wiring existing fields into the chromatogram shape. + // + // This is deliberately distinct from the per-spectrum + // `SpectrumRecord.total_ion_current` field, which `iter_spectra` + // leaves `None` on purpose: the conformance suite checks that field + // with `rel_close` against `sum(raw intensities)`, and the Idx TIC is + // in cps (physically calibrated) so it would not match. That check + // applies only to the per-spectrum field; a TIC chromatogram is a + // separate trace of (retention time, cps) points that the conformance + // suite never inspects, so the mismatch reasoning does not apply here. + // + // Only TIC is emitted. A basepeak chromatogram (BPC) would need a + // per-scan base peak, which nothing decodes today, and SRM/MRM + // chromatograms would need transition-level data that this reader + // does not decode at all - both are tracked as separate follow-ups + // (see Sigilweaver/OpenSXRaw#21). + let mut time_sec = Vec::with_capacity(self.idx_records.len()); + let mut intensity = Vec::with_capacity(self.idx_records.len()); + for rec in &self.idx_records { + time_sec.push(rec.retention_time_min * 60.0); + intensity.push(rec.tic as f32); + } + + let record = ChromatogramRecord { + index: 0, + id: "TIC".to_string(), + chromatogram_type: Some(CvTerm::new("MS:1000235", "total ion current chromatogram")), + precursor_mz: None, + product_mz: None, + time_sec, + intensity, + }; + + Box::new(std::iter::once(record)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::path::PathBuf; + + /// Build a `Reader` from synthetic `idx_records` only, bypassing file I/O, + /// so the chromatogram wiring can be exercised without a corpus fixture. + /// This mirrors what `Reader::open` populates: `iter_chromatograms` reads + /// only `idx_records`, so the remaining fields are inert placeholders. + fn reader_with_idx(idx_records: Vec) -> Reader { + Reader { + stem: "synthetic".to_string(), + scan_path: PathBuf::from("synthetic.wiff.scan"), + idx_records, + scan_file_size: 0, + start_timestamp: None, + calibration: None, + dde_records: Vec::new(), + } + } + + fn idx_record(retention_time_min: f32, ms_level: u32, tic: f64) -> IdxRecord { + IdxRecord { + scan_offset: 0, + scan_size: 0, + retention_time_min, + ms_level, + tic, + _field_1a: 0.0, + } + } + + #[test] + fn iter_chromatograms_emits_single_tic_from_idx_records() { + // One MS1 and one MS2 record: the TIC chromatogram has one point per + // Idx record regardless of MS level. + let mut reader = + reader_with_idx(vec![idx_record(0.0, 1, 100.0), idx_record(0.5, 2, 250.0)]); + + let chroms: Vec = reader.iter_chromatograms().collect(); + + assert_eq!(chroms.len(), 1, "expected exactly one TIC chromatogram"); + let tic = &chroms[0]; + assert_eq!(tic.index, 0); + assert_eq!(tic.id, "TIC"); + let cv = tic + .chromatogram_type + .as_ref() + .expect("TIC chromatogram must carry a chromatogram_type CV term"); + assert_eq!(cv.accession, "MS:1000235"); + assert_eq!(cv.name, "total ion current chromatogram"); + + // TIC carries no precursor/product m/z (those are for SRM/MRM). + assert!(tic.precursor_mz.is_none()); + assert!(tic.product_mz.is_none()); + + // time_sec is retention_time_min * 60.0; intensity is the Idx tic. + assert_eq!(tic.time_sec, vec![0.0, 30.0]); + assert_eq!(tic.intensity, vec![100.0, 250.0]); + assert_eq!(tic.time_sec.len(), tic.intensity.len()); + } + + #[test] + fn iter_chromatograms_tic_intensity_uses_idx_tic_not_spectrum_field() { + // Regression guard for the reasoning in the method's doc comment: the + // TIC chromatogram intensity comes straight from the Idx record's + // physically-calibrated `tic` (cps), independent of the per-spectrum + // `SpectrumRecord.total_ion_current` field (which stays `None` so the + // conformance suite's `rel_close` check against sum(intensities) is + // not tripped). A large cps value that could never equal a raw + // intensity sum must still pass through verbatim. + let mut reader = reader_with_idx(vec![idx_record(1.0, 1, 1_234_567.0)]); + let chroms: Vec = reader.iter_chromatograms().collect(); + assert_eq!(chroms.len(), 1); + assert_eq!(chroms[0].time_sec, vec![60.0]); + assert_eq!(chroms[0].intensity, vec![1_234_567.0]); + } } diff --git a/crates/opensxraw/tests/conformance.rs b/crates/opensxraw/tests/conformance.rs index a66116b..c541909 100644 --- a/crates/opensxraw/tests/conformance.rs +++ b/crates/opensxraw/tests/conformance.rs @@ -81,6 +81,35 @@ fn test_conformance_invariants() { println!("Conformance passed: {} spectra", n); } +#[test] +fn test_iter_chromatograms_emits_tic() { + let Some(mut reader) = open_fixture_or_skip() else { + return; + }; + let n_records = reader.idx_records.len(); + assert!(n_records > 0, "fixture should have Idx records"); + + let chroms: Vec<_> = reader.iter_chromatograms().collect(); + + // Exactly one TIC chromatogram: BPC/SRM need net-new decode work and are + // intentionally out of scope (see Sigilweaver/OpenSXRaw#21). + assert_eq!(chroms.len(), 1, "expected exactly one (TIC) chromatogram"); + let tic = &chroms[0]; + let cv = tic + .chromatogram_type + .as_ref() + .expect("TIC chromatogram must carry a chromatogram_type CV term"); + assert_eq!(cv.accession, "MS:1000235"); + assert_eq!(cv.name, "total ion current chromatogram"); + assert!(tic.precursor_mz.is_none()); + assert!(tic.product_mz.is_none()); + + // One point per Idx record, with parallel time/intensity arrays. + assert_eq!(tic.time_sec.len(), n_records); + assert_eq!(tic.intensity.len(), n_records); + println!("TIC chromatogram: {} points", tic.time_sec.len()); +} + #[test] fn test_ms1_has_peaks() { let Some(mut reader) = open_fixture_or_skip() else {