From 6f3414fdbf2f04fec1a7581db3a995790f530da6 Mon Sep 17 00:00:00 2001 From: Benjamin Demaille Date: Tue, 28 Jul 2026 21:10:14 +0200 Subject: [PATCH 1/5] test(cli): lock the STAR 2.7.11b parameter surface, and declare the 31 knobs this PR covers Adds `tests/parameter_surface.rs` and the name list it checks against. The list is the column-1 names of STAR 2.7.11b's `parametersDefault`, minus the three non-user meta-parameters (`parametersFiles`, `sysShell`, `versionGenome`): 200 names, the public CLI surface only, no STAR source. The test asserts every one of those names is either recognised by clap or listed in `NOT_YET_ACCEPTED`, so the coverage figure is machine-checked rather than claimed, and surface drift fails a test the moment it happens instead of turning up in a bug report. It also asserts the reverse: a name listed as not accepted that has since been implemented must be removed from the list. Two supporting lists keep "accepted" from quietly meaning "implemented": - `ACCEPTED_BUT_INERT` names the 16 knobs that parse but change no output byte, each with the reason. A second test checks the CLI really does accept them, so the annotation cannot drift into being a lie. - `NOT_YET_ACCEPTED` names the 26 still missing, grouped by the theme that will bring them, so the remaining gap is legible rather than a bare number. This commit declares the 31 parameters in this PR's scope. Behaviour follows in the next commits; the ones that will stay inert are already annotated as such. Surface goes from 143/200 to 174/200 accepted. Co-Authored-By: Claude Opus 5 (1M context) --- src/params/mod.rs | 141 ++++++++++++++++++ tests/data/star_2.7.11b_params.txt | 205 ++++++++++++++++++++++++++ tests/parameter_surface.rs | 226 +++++++++++++++++++++++++++++ 3 files changed, 572 insertions(+) create mode 100644 tests/data/star_2.7.11b_params.txt create mode 100644 tests/parameter_surface.rs diff --git a/src/params/mod.rs b/src/params/mod.rs index 0536a85..6eaea55 100644 --- a/src/params/mod.rs +++ b/src/params/mod.rs @@ -528,6 +528,40 @@ pub struct Parameters { #[arg(long = "readFilesCommand")] pub read_files_command: Option, + /// Prefix prepended to every path in `--readFilesIn`. + #[arg(long = "readFilesPrefix", default_value = "")] + pub read_files_prefix: String, + + /// Input format: `Fastx` (default, FASTA/FASTQ). + #[arg(long = "readFilesType", num_args = 1..=2, default_values_t = vec!["Fastx".to_string()])] + pub read_files_type: Vec, + + /// SAM tag holding the barcode sequence when reading aligned input. + #[arg(long = "soloInputSAMattrBarcodeSeq", num_args = 1.., default_values_t = vec!["-".to_string()])] + pub solo_input_sam_attr_barcode_seq: Vec, + + /// SAM tag holding the barcode qualities when reading aligned input. + #[arg(long = "soloInputSAMattrBarcodeQual", num_args = 1.., default_values_t = vec!["-".to_string()])] + pub solo_input_sam_attr_barcode_qual: Vec, + + /// SAM attributes to carry over when reading aligned input. + #[arg(long = "readFilesSAMattrKeep", num_args = 1.., default_values_t = vec!["All".to_string()])] + pub read_files_sam_attr_keep: Vec, + + /// Characters that terminate a read name. Everything from the first + /// occurrence of any of these is dropped. `-` keeps the whole name. + #[arg(long = "readNameSeparator", num_args = 1.., default_values_t = vec!["/".to_string()])] + pub read_name_separator: Vec, + + /// Phred offset of the input quality strings (33 or 64). 0 selects 33. + #[arg(long = "readQualityScoreBase", default_value_t = 33)] + pub read_quality_score_base: i32, + + /// Read lengths declared up front, when they are not to be taken from the + /// input. + #[arg(long = "readMatesLengthsIn", default_value = "NotEqual")] + pub read_mates_lengths_in: String, + /// `--soloType SmartSeq` manifest: a TSV with `read1 read2 cellID` /// per line (`read2` = `-` for single-end). Each line is one plate-well cell; /// reads are counted per gene with no UMI. @@ -626,11 +660,118 @@ pub struct Parameters { #[arg(long = "limitGenomeGenerateRAM", default_value = "31G", value_parser = parse_mem_bytes)] pub limit_genome_generate_ram: u64, + /// Size of the I/O buffers, as input and output byte counts. + #[arg(long = "limitIObufferSize", num_args = 1..=2, + default_values_t = vec![30_000_000u64, 50_000_000u64])] + pub limit_io_buffer_size: Vec, + + /// Soft limit on the number of reads processed. + #[arg(long = "limitNreadsSoft", default_value_t = -1i64, allow_hyphen_values = true)] + pub limit_nreads_soft: i64, + + /// Maximum size in bytes of the SAM records emitted for one read. + #[arg(long = "limitOutSAMoneReadBytes", default_value_t = 100_000u64)] + pub limit_out_sam_one_read_bytes: u64, + + /// Maximum number of collapsed junctions. + #[arg(long = "limitOutSJcollapsed", default_value_t = 1_000_000u64)] + pub limit_out_sj_collapsed: u64, + + /// Maximum number of junctions recorded for one read. + #[arg(long = "limitOutSJoneRead", default_value_t = 1_000u64)] + pub limit_out_sj_one_read: u64, + + /// Maximum number of junctions inserted on the fly. + #[arg(long = "limitSjdbInsertNsj", default_value_t = 1_000_000u64)] + pub limit_sjdb_insert_nsj: u64, + + /// Permissions for directories created by the run. + #[arg(long = "runDirPerm", default_value = "User_RWX")] + pub run_dir_perm: String, + + /// Declared sizes of the genome files. + #[arg(long = "genomeFileSizes", num_args = 1.., default_values_t = vec![0u64])] + pub genome_file_sizes: Vec, + /// Route primary alignment output to stdout instead of a file. /// Values: None (default), SAM, BAM_Unsorted, BAM_SortedByCoordinate. #[arg(long = "outStd", default_value = "None")] pub out_std: OutStd, + /// Alignment output mode: `Full` (default), `NoQS` (omit quality strings) + /// or `None` (no alignment output at all). + #[arg(long = "outSAMmode", default_value = "Full")] + pub out_sam_mode: String, + + /// Order of alignments in the output: `Paired` (default) or + /// `PairedKeepInputOrder`. + #[arg(long = "outSAMorder", default_value = "Paired")] + pub out_sam_order: String, + + /// Post-alignment record filter. Only the default (no filtering) is + /// supported; the added-reference modes need align-time reference + /// insertion, which this aligner does not do. + #[arg(long = "outSAMfilter", num_args = 1.., default_values_t = vec!["None".to_string()])] + pub out_sam_filter: Vec, + + /// `@HD` header line, given as its tab-separated fields. + #[arg(long = "outSAMheaderHD", num_args = 1..)] + pub out_sam_header_hd: Vec, + + /// Extra `@PG` header line, given as its tab-separated fields. + #[arg(long = "outSAMheaderPG", num_args = 1..)] + pub out_sam_header_pg: Vec, + + /// File whose lines are emitted as `@CO` header comments. + #[arg(long = "outSAMheaderCommentFile", default_value = "-")] + pub out_sam_header_comment_file: String, + + /// Added to every output quality score. Use -31 to convert Phred+64 input + /// to Phred+33 output. + #[arg( + long = "outQSconversionAdd", + default_value_t = 0, + allow_hyphen_values = true + )] + pub out_qs_conversion_add: i32, + + /// Splice-junction output: `Standard` (default) writes SJ.out.tab, `None` + /// suppresses it. + #[arg(long = "outSJtype", default_value = "Standard")] + pub out_sj_type: String, + + /// Which reads contribute to SJ.out.tab: `All` (default) or `Unique`. + #[arg(long = "outSJfilterReads", default_value = "All")] + pub out_sj_filter_reads: String, + + /// Prefix for reference names in signal output. + #[arg(long = "outWigReferencesPrefix", default_value = "-")] + pub out_wig_references_prefix: String, + + /// Directory for intermediate files. + #[arg(long = "outTmpDir", default_value = "-")] + pub out_tmp_dir: String, + + /// Keep intermediate files after the run. + #[arg(long = "outTmpKeep", default_value = "None")] + pub out_tmp_keep: String, + + /// Number of bins used when sorting BAM by coordinate. + #[arg(long = "outBAMsortingBinsN", default_value_t = 50)] + pub out_bam_sorting_bins_n: usize, + + /// Threads used for BAM sorting. 0 selects `--runThreadN`. + #[arg(long = "outBAMsortingThreadN", default_value_t = 0)] + pub out_bam_sorting_thread_n: usize, + + /// gzip level for the transcriptome BAM, -1 to 10. + #[arg( + long = "quantTranscriptomeBAMcompression", + default_value_t = 1, + allow_hyphen_values = true + )] + pub quant_transcriptome_bam_compression: i32, + /// Strand field: None or intronMotif #[arg(long = "outSAMstrandField", default_value = "None")] pub out_sam_strand_field: String, diff --git a/tests/data/star_2.7.11b_params.txt b/tests/data/star_2.7.11b_params.txt new file mode 100644 index 0000000..66edbae --- /dev/null +++ b/tests/data/star_2.7.11b_params.txt @@ -0,0 +1,205 @@ +# STAR 2.7.11b user-facing parameter names (the public CLI surface, not implementation). +# Derived from the column-1 names of STAR 2.7.11b's parametersDefault, minus the three +# non-user meta-params parametersFiles, sysShell, versionGenome. This is a NAME list only +# (the public interface), never STAR source; it locks the CLI parameter-surface parity via +# tests/parameter_surface.rs. 200 names. +alignEndsProtrude +alignEndsType +alignInsertionFlush +alignIntronMax +alignIntronMin +alignMatesGapMax +alignSJDBoverhangMin +alignSJoverhangMin +alignSJstitchMismatchNmax +alignSoftClipAtReferenceEnds +alignSplicedMateMapLmin +alignSplicedMateMapLminOverLmate +alignTranscriptsPerReadNmax +alignTranscriptsPerWindowNmax +alignWindowsPerReadNmax +bamRemoveDuplicatesMate2basesN +bamRemoveDuplicatesType +chimFilter +chimJunctionOverhangMin +chimMainSegmentMultNmax +chimMultimapNmax +chimMultimapScoreRange +chimNonchimScoreDropMin +chimOutJunctionFormat +chimOutType +chimScoreDropMax +chimScoreJunctionNonGTAG +chimScoreMin +chimScoreSeparation +chimSegmentMin +chimSegmentReadGapMax +clip3pAdapterMMp +clip3pAdapterSeq +clip3pAfterAdapterNbases +clip3pNbases +clip5pAdapterMMp +clip5pAdapterSeq +clip5pAfterAdapterNbases +clip5pNbases +clipAdapterType +genomeChainFiles +genomeChrBinNbits +genomeChrSetMitochondrial +genomeDir +genomeFastaFiles +genomeFileSizes +genomeLoad +genomeSAindexNbases +genomeSAsparseD +genomeSuffixLengthMax +genomeTransformOutput +genomeTransformType +genomeTransformVCF +genomeType +inputBAMfile +limitBAMsortRAM +limitGenomeGenerateRAM +limitIObufferSize +limitNreadsSoft +limitOutSAMoneReadBytes +limitOutSJcollapsed +limitOutSJoneRead +limitSjdbInsertNsj +outBAMcompression +outBAMsortingBinsN +outBAMsortingThreadN +outFileNamePrefix +outFilterIntronMotifs +outFilterIntronStrands +outFilterMatchNmin +outFilterMatchNminOverLread +outFilterMismatchNmax +outFilterMismatchNoverLmax +outFilterMismatchNoverReadLmax +outFilterMultimapNmax +outFilterMultimapScoreRange +outFilterScoreMin +outFilterScoreMinOverLread +outFilterType +outMultimapperOrder +outQSconversionAdd +outReadsUnmapped +outSAMattrIHstart +outSAMattrRGline +outSAMattributes +outSAMfilter +outSAMflagAND +outSAMflagOR +outSAMheaderCommentFile +outSAMheaderHD +outSAMheaderPG +outSAMmapqUnique +outSAMmode +outSAMmultNmax +outSAMorder +outSAMprimaryFlag +outSAMreadID +outSAMstrandField +outSAMtlen +outSAMtype +outSAMunmapped +outSJfilterCountTotalMin +outSJfilterCountUniqueMin +outSJfilterDistToOtherSJmin +outSJfilterIntronMaxVsReadN +outSJfilterOverhangMin +outSJfilterReads +outSJtype +outStd +outTmpDir +outTmpKeep +outWigNorm +outWigReferencesPrefix +outWigStrand +outWigType +peOverlapMMp +peOverlapNbasesMin +quantMode +quantTranscriptomeBAMcompression +quantTranscriptomeSAMoutput +readFilesCommand +readFilesIn +readFilesManifest +readFilesPrefix +readFilesSAMattrKeep +readFilesType +readMapNumber +readMatesLengthsIn +readNameSeparator +readQualityScoreBase +runDirPerm +runMode +runRNGseed +runThreadN +scoreDelBase +scoreDelOpen +scoreGap +scoreGapATAC +scoreGapGCAG +scoreGapNoncan +scoreGenomicLengthLog2scale +scoreInsBase +scoreInsOpen +scoreStitchSJshift +seedMapMin +seedMultimapNmax +seedNoneLociPerWindow +seedPerReadNmax +seedPerWindowNmax +seedSearchLmax +seedSearchStartLmax +seedSearchStartLmaxOverLread +seedSplitMin +sjdbFileChrStartEnd +sjdbGTFchrPrefix +sjdbGTFfeatureExon +sjdbGTFfile +sjdbGTFtagExonParentGene +sjdbGTFtagExonParentGeneName +sjdbGTFtagExonParentGeneType +sjdbGTFtagExonParentTranscript +sjdbInsertSave +sjdbOverhang +sjdbScore +soloAdapterMismatchesNmax +soloAdapterSequence +soloBarcodeMate +soloBarcodeReadLength +soloCBlen +soloCBmatchWLtype +soloCBposition +soloCBstart +soloCBtype +soloCBwhitelist +soloCellFilter +soloCellReadStats +soloClusterCBfile +soloFeatures +soloInputSAMattrBarcodeQual +soloInputSAMattrBarcodeSeq +soloMultiMappers +soloOutFileNames +soloOutFormatFeaturesGeneField3 +soloStrand +soloType +soloUMIdedup +soloUMIfiltering +soloUMIlen +soloUMIposition +soloUMIstart +twopass1readsN +twopassMode +varVCFfile +waspOutputMode +winAnchorDistNbins +winAnchorMultimapNmax +winBinNbits +winFlankNbins +winReadCoverageBasesMin +winReadCoverageRelativeMin diff --git a/tests/parameter_surface.rs b/tests/parameter_surface.rs new file mode 100644 index 0000000..ff579c1 --- /dev/null +++ b/tests/parameter_surface.rs @@ -0,0 +1,226 @@ +//! Locks the CLI parameter surface against STAR 2.7.11b. +//! +//! `tests/data/star_2.7.11b_params.txt` lists every user-facing parameter name +//! STAR 2.7.11b accepts. This test asserts that clap recognises each one, so +//! the coverage figure is a machine-checked number rather than a claim, and so +//! surface drift shows up the moment it happens rather than in a bug report. +//! +//! A recognised name here means "the CLI accepts it and does not error", not +//! "it is fully implemented". Parameters that are deliberately accepted but +//! output-neutral are listed in `ACCEPTED_BUT_INERT` below, with the reason. +//! Parameters not yet accepted at all live in `NOT_YET_ACCEPTED`, which is the +//! honest remaining gap. + +use std::collections::BTreeSet; + +/// Names accepted by the CLI but with no effect on output, each for a stated +/// reason. Keeping the list explicit stops "accepted" from quietly meaning +/// "implemented". +const ACCEPTED_BUT_INERT: &[(&str, &str)] = &[ + ( + "genomeFileSizes", + "an input hint for STAR's loader; no output bytes depend on it", + ), + ( + "limitBAMsortRAM", + "sorting is bounded by the writer, not by a byte budget", + ), + ( + "limitGenomeGenerateRAM", + "genome generation manages its own memory", + ), + ( + "limitIObufferSize", + "buffer sizing is an implementation detail with no output effect", + ), + ( + "limitNreadsSoft", + "a soft warning threshold in STAR; no output bytes depend on it", + ), + ( + "limitOutSAMoneReadBytes", + "an overflow guard on STAR's fixed output buffer", + ), + ( + "limitOutSJcollapsed", + "an allocation cap on STAR's collapsed-junction array", + ), + ( + "limitOutSJoneRead", + "an allocation cap on STAR's per-read junction array", + ), + ( + "limitSjdbInsertNsj", + "an allocation cap on the inserted-junction array", + ), + ( + "outBAMsortingBinsN", + "sorting is not binned yet; output is unaffected", + ), + ( + "outBAMsortingThreadN", + "BGZF writing is single-threaded; output is unaffected", + ), + ( + "outTmpDir", + "no intermediate files are written that a user could observe", + ), + ("outTmpKeep", "as outTmpDir: nothing to keep"), + ( + "readMatesLengthsIn", + "a read-length hint; lengths are taken from the FASTQ", + ), + ( + "runDirPerm", + "no directories are created whose mode a user could observe", + ), + ( + "runRNGseed", + "multimapper selection is seeded per read, not from a global RNG", + ), +]; + +/// Names STAR accepts that this CLI does not yet accept at all. This is the +/// real remaining surface gap; shrinking it is the point of the port. +/// +/// Adding a name here must always be a deliberate act. Removing one is what +/// progress looks like. +const NOT_YET_ACCEPTED: &[&str] = &[ + // Aligner core (annotated-junction stitching, alignEndsType, in-recursion + // length penalty). + "alignEndsProtrude", + "alignEndsType", + "alignInsertionFlush", + "alignSoftClipAtReferenceEnds", + "alignTranscriptsPerReadNmax", + "outFilterMismatchNoverReadLmax", + "seedNoneLociPerWindow", + "seedSplitMin", + // Long reads. + "winReadCoverageBasesMin", + // Chimeric multimapping. + "chimFilter", + "chimMultimapNmax", + "chimMultimapScoreRange", + "chimNonchimScoreDropMin", + // CellRanger4 adapter clipping. + "clip5pAdapterMMp", + "clip5pAdapterSeq", + // Genome index types and transforms. + "genomeChrSetMitochondrial", + "genomeSuffixLengthMax", + "genomeTransformOutput", + "genomeType", + "sjdbInsertSave", + // STARsolo barcode chemistry. + "soloAdapterMismatchesNmax", + "soloAdapterSequence", + "soloCBtype", + "soloOutFormatFeaturesGeneField3", + // STARsolo Transcript3p / CellReads.stats / cell filtering. + "soloCellReadStats", + "soloClusterCBfile", +]; + +fn star_parameter_names() -> Vec { + let raw = include_str!("data/star_2.7.11b_params.txt"); + raw.lines() + .map(str::trim) + .filter(|l| !l.is_empty() && !l.starts_with('#')) + .map(str::to_string) + .collect() +} + +/// Every long flag clap knows about, walked recursively so flattened argument +/// groups are included. +fn recognised_flags() -> BTreeSet { + use clap::CommandFactory; + fn walk(cmd: &clap::Command, out: &mut BTreeSet) { + for arg in cmd.get_arguments() { + if let Some(long) = arg.get_long() { + out.insert(long.to_string()); + } + for alias in arg.get_all_aliases().unwrap_or_default() { + out.insert(alias.to_string()); + } + } + for sub in cmd.get_subcommands() { + walk(sub, out); + } + } + let cmd = rustar_aligner::params::Parameters::command(); + let mut out = BTreeSet::new(); + walk(&cmd, &mut out); + out +} + +#[test] +fn star_parameter_surface_is_fully_accounted_for() { + let star = star_parameter_names(); + let ours = recognised_flags(); + let not_yet: BTreeSet<&str> = NOT_YET_ACCEPTED.iter().copied().collect(); + + let mut unexpected_missing = Vec::new(); + let mut unexpectedly_present = Vec::new(); + + for name in &star { + let known = ours.contains(name.as_str()); + let declared_missing = not_yet.contains(name.as_str()); + if !known && !declared_missing { + unexpected_missing.push(name.clone()); + } + if known && declared_missing { + unexpectedly_present.push(name.clone()); + } + } + + assert!( + unexpected_missing.is_empty(), + "{} STAR parameter(s) are not recognised and are not declared in \ + NOT_YET_ACCEPTED. Either implement them or add them to that list \ + deliberately:\n {}", + unexpected_missing.len(), + unexpected_missing.join("\n ") + ); + + assert!( + unexpectedly_present.is_empty(), + "{} parameter(s) are now recognised but still listed in \ + NOT_YET_ACCEPTED; remove them from that list:\n {}", + unexpectedly_present.len(), + unexpectedly_present.join("\n ") + ); +} + +#[test] +fn inert_parameters_are_actually_accepted() { + // A name in ACCEPTED_BUT_INERT that the CLI does not accept would be a + // documentation lie, so check the claim rather than trusting it. + let ours = recognised_flags(); + let missing: Vec<&str> = ACCEPTED_BUT_INERT + .iter() + .map(|(name, _)| *name) + .filter(|name| !ours.contains(*name)) + .collect(); + assert!( + missing.is_empty(), + "listed as accepted-but-inert, but not accepted: {missing:?}" + ); +} + +#[test] +fn report_coverage() { + // Not an assertion: prints the machine-checked coverage figure so it can be + // quoted without anyone counting by hand. + let star = star_parameter_names(); + let ours = recognised_flags(); + let covered = star.iter().filter(|n| ours.contains(n.as_str())).count(); + println!( + "STAR 2.7.11b parameter surface: {covered}/{} accepted ({} inert), \ + {} not yet accepted", + star.len(), + ACCEPTED_BUT_INERT.len(), + star.len() - covered, + ); + assert!(covered > 0); +} From f5a232910f1d1b7ab027d12b2a3f9ab0c04d9e94 Mon Sep 17 00:00:00 2001 From: Benjamin Demaille Date: Tue, 28 Jul 2026 21:20:24 +0200 Subject: [PATCH 2/5] feat(cli): SAM/SJ/read-input knobs, and cut read names at the separator as STAR does Implements the behavioural half of this theme. SAM output: - `--outSAMmode None` routes to the existing null writer, so alignment still runs and SJ.out.tab, Log.final.out and the count matrices are still produced; only the records are dropped. `NoQS` strips quality strings through a small decorator around the boxed writer, rather than at each of the five record-building sites, so the clone is paid only when the flag is set. - `--outSAMheaderHD`, `--outSAMheaderPG` and `--outSAMheaderCommentFile` build the @HD, extra @PG and @CO lines, with TAG:value fields validated rather than passed through. Splice junctions: - `--outSJtype None` suppresses SJ.out.tab. - `--outSJfilterReads Unique` counts only uniquely-mapping reads towards the outSJfilter* thresholds, so a junction supported solely by multimappers drops out. Read input: - `--readFilesPrefix` is applied during parsing, so every consumer sees final paths. - `--readQualityScoreBase` and `--outQSconversionAdd` become a single signed shift applied as the FASTQ is read, normalising to Phred+33 once. - `--readNameSeparator` cuts read names at the first separator. **One output change at default settings.** STAR's default `--readNameSeparator` is `/`, and STAR cuts there. rustar-aligner did not, so a read named `foo/1` was emitted as `foo/1` where STAR emits `foo`. Verified against native STAR 2.7.11b: STAR sim_0_YDR305C_mRNA_j567 origin/main sim_0_YDR305C_mRNA_j567/1 this branch sim_0_YDR305C_mRNA_j567 The unit test that asserted the old behaviour has been updated; it was encoding the divergence. Reads whose names contain no separator are unaffected, which is why the 1000-read differential set is byte-identical to origin/main. Loud rejections rather than silent acceptance: - `--outSAMfilter KeepOnlyAddedReferences` / `KeepAllAddedReferences` need align-time `--genomeFastaFiles` reference insertion, which this aligner does not do. - `--readFilesType SAM SE|PE` is refused rather than silently treating a SAM file as FASTQ. `--outSAMorder` is accepted at both values: `run_batch_pipeline` already consumes batches in input order, so `PairedKeepInputOrder` is satisfied. Co-Authored-By: Claude Opus 5 (1M context) --- src/io/fastq.rs | 69 +++++++++++++++- src/io/sam.rs | 76 +++++++++++++++++- src/junction/sj_output.rs | 20 ++++- src/lib.rs | 164 ++++++++++++++++++++++++-------------- src/params/mod.rs | 94 ++++++++++++++++++++++ 5 files changed, 356 insertions(+), 67 deletions(-) diff --git a/src/io/fastq.rs b/src/io/fastq.rs index 71b1f81..0c795de 100644 --- a/src/io/fastq.rs +++ b/src/io/fastq.rs @@ -72,6 +72,19 @@ pub struct PairedRead { /// FASTQ reader that handles decompression and base encoding pub struct FastqReader { inner: fastq::io::Reader>, + /// Signed shift applied to every input quality byte so the rest of the + /// pipeline always sees Phred+33. + /// + /// `--readQualityScoreBase 64` contributes `-31`, converting Solexa/Illumina + /// 1.3 input. `--outQSconversionAdd` contributes its own value, which STAR + /// documents as an output conversion; applying it here rather than at the + /// writer means it also reaches anything else that reads qualities. That is + /// only observable when both it and STARsolo are in use, which STAR itself + /// does not support either. + qual_shift: i32, + /// Characters that terminate a read name (`--readNameSeparator`). Empty + /// means keep the whole name. + name_separators: Vec, } impl FastqReader { @@ -115,9 +128,30 @@ impl FastqReader { Ok(Self { inner: fastq_reader, + qual_shift: 0, + name_separators: vec![b'/'], }) } + /// Apply the read-input knobs: `--readQualityScoreBase`, + /// `--outQSconversionAdd` and `--readNameSeparator`. + #[must_use] + pub fn with_params(mut self, params: &crate::params::Parameters) -> Self { + let base = if params.read_quality_score_base == 0 { + 33 + } else { + params.read_quality_score_base + }; + self.qual_shift = (33 - base) + params.out_qs_conversion_add; + self.name_separators = params + .read_name_separator + .iter() + .filter(|s| s.as_str() != "-") + .filter_map(|s| s.as_bytes().first().copied()) + .collect(); + self + } + /// Open FASTQ file using external decompression command fn open_with_command(path: &Path, cmd: &str) -> Result, Error> { let mut child = Command::new(cmd) @@ -150,7 +184,25 @@ impl FastqReader { let sequence = record.sequence().iter().map(|&b| encode_base(b)).collect(); - let quality = record.quality_scores().to_vec(); + let name = match self + .name_separators + .iter() + .filter_map(|&sep| name.as_bytes().iter().position(|&b| b == sep)) + .min() + { + Some(cut) => name[..cut].to_string(), + None => name, + }; + + let quality = if self.qual_shift == 0 { + record.quality_scores().to_vec() + } else { + record + .quality_scores() + .iter() + .map(|&b| (b as i32 + self.qual_shift).clamp(33, 126) as u8) + .collect() + }; Ok(Some(EncodedRead { name, @@ -205,6 +257,15 @@ impl PairedFastqReader { Ok(Self { reader1, reader2 }) } + /// Apply the read-input knobs to both mates. See + /// [`FastqReader::with_params`]. + #[must_use] + pub fn with_params(mut self, params: &crate::params::Parameters) -> Self { + self.reader1 = self.reader1.with_params(params); + self.reader2 = self.reader2.with_params(params); + self + } + /// Get next paired read with name validation /// /// # Returns @@ -565,9 +626,11 @@ mod tests { let pair1 = reader.next_paired().unwrap().unwrap(); assert_eq!(pair1.name, "read1"); - assert_eq!(pair1.mate1.name, "read1/1"); + // Read names are cut at `/`, STAR's default --readNameSeparator, so + // both mates report the shared name rather than the /1 and /2 forms. + assert_eq!(pair1.mate1.name, "read1"); assert_eq!(pair1.mate1.sequence, vec![0, 1, 2, 3]); // ACGT - assert_eq!(pair1.mate2.name, "read1/2"); + assert_eq!(pair1.mate2.name, "read1"); assert_eq!(pair1.mate2.sequence, vec![2, 2, 1, 1]); // GGCC let pair2 = reader.next_paired().unwrap().unwrap(); diff --git a/src/io/sam.rs b/src/io/sam.rs index 588c71d..5dc3068 100644 --- a/src/io/sam.rs +++ b/src/io/sam.rs @@ -950,8 +950,35 @@ where { let mut builder = sam::Header::builder(); - // @HD line (default version and unsorted) - builder = builder.set_header(Map::default()); + // @HD line. `--outSAMheaderHD` replaces it wholesale, given as the + // tab-separated fields STAR expects (`@HD VN:1.4 SO:coordinate`). + if params.out_sam_header_hd.is_empty() { + builder = builder.set_header(Map::default()); + } else { + let mut hd = Map::::default(); + for field in ¶ms.out_sam_header_hd { + let field = field.trim(); + // STAR takes the leading `@HD` as part of the value list; ignore it. + if field.is_empty() || field == "@HD" { + continue; + } + let Some((tag, value)) = field.split_once(':') else { + return Err(Error::Parameter(format!( + "--outSAMheaderHD field '{field}' is not TAG:value" + ))); + }; + if tag.len() != 2 { + return Err(Error::Parameter(format!( + "--outSAMheaderHD tag '{tag}' is not two characters" + ))); + } + let tag_bytes: [u8; 2] = tag.as_bytes()[..2].try_into().unwrap(); + let other_tag: HeaderOtherTag<_> = HeaderOtherTag::try_from(tag_bytes) + .map_err(|e| Error::Parameter(format!("invalid @HD tag '{tag}': {e}")))?; + hd.other_fields_mut().insert(other_tag, value.into()); + } + builder = builder.set_header(hd); + } // @SQ lines for each reference for (name, length) in refs { @@ -1009,6 +1036,51 @@ where .insert(program_tag::COMMAND_LINE, BString::from(cl)); builder = builder.add_program("rustar-aligner", pg); + // `--outSAMheaderPG` adds one further @PG line, given the same way. + if !params.out_sam_header_pg.is_empty() { + let mut extra = Map::::default(); + let mut id: Option = None; + for field in ¶ms.out_sam_header_pg { + let field = field.trim(); + if field.is_empty() || field == "@PG" { + continue; + } + let Some((tag, value)) = field.split_once(':') else { + return Err(Error::Parameter(format!( + "--outSAMheaderPG field '{field}' is not TAG:value" + ))); + }; + if tag == "ID" { + id = Some(value.to_string()); + continue; + } + if tag.len() != 2 { + return Err(Error::Parameter(format!( + "--outSAMheaderPG tag '{tag}' is not two characters" + ))); + } + let tag_bytes: [u8; 2] = tag.as_bytes()[..2].try_into().unwrap(); + let other_tag: HeaderOtherTag<_> = HeaderOtherTag::try_from(tag_bytes) + .map_err(|e| Error::Parameter(format!("invalid @PG tag '{tag}': {e}")))?; + extra.other_fields_mut().insert(other_tag, value.into()); + } + let id = + id.ok_or_else(|| Error::Parameter("--outSAMheaderPG needs an ID: field".to_string()))?; + builder = builder.add_program(id, extra); + } + + // `--outSAMheaderCommentFile` contributes one @CO line per line of the + // named file. `-` (the default) means no comments. + if params.out_sam_header_comment_file != "-" { + let path = std::path::Path::new(¶ms.out_sam_header_comment_file); + let contents = std::fs::read_to_string(path).map_err(|e| Error::io(e, path))?; + for line in contents.lines() { + if !line.is_empty() { + builder = builder.add_comment(line); + } + } + } + Ok(builder.build()) } diff --git a/src/junction/sj_output.rs b/src/junction/sj_output.rs index 9fa41dd..fcc58d6 100644 --- a/src/junction/sj_output.rs +++ b/src/junction/sj_output.rs @@ -132,11 +132,19 @@ impl SpliceJunctionStats { .map(|entry| { let key = entry.key().clone(); let counts = entry.value(); + // `--outSJfilterReads Unique` counts only uniquely-mapping + // reads towards the thresholds, so a junction supported solely + // by multimappers drops out. + let multi = if params.out_sj_filter_reads == "Unique" { + 0 + } else { + counts.multi_count.load(Ordering::Relaxed) + }; ( key, counts.annotated, counts.unique_count.load(Ordering::Relaxed), - counts.multi_count.load(Ordering::Relaxed), + multi, counts.max_overhang.load(Ordering::Relaxed), ) }) @@ -281,11 +289,19 @@ impl SpliceJunctionStats { .map(|entry| { let key = entry.key().clone(); let counts = entry.value(); + // `--outSJfilterReads Unique` counts only uniquely-mapping + // reads towards the thresholds, so a junction supported solely + // by multimappers drops out. + let multi = if params.out_sj_filter_reads == "Unique" { + 0 + } else { + counts.multi_count.load(Ordering::Relaxed) + }; ( key, counts.annotated, counts.unique_count.load(Ordering::Relaxed), - counts.multi_count.load(Ordering::Relaxed), + multi, counts.max_overhang.load(Ordering::Relaxed), ) }) diff --git a/src/lib.rs b/src/lib.rs index 6fce173..3512503 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -153,6 +153,31 @@ trait AlignmentWriter: Send { } } +/// Drops quality strings on the way to the real writer (`--outSAMmode NoQS`). +/// +/// A decorator rather than a change at every record-building site: the records +/// are built once and consumed by five different writer types, and only this +/// flag cares. The clone is paid only when the flag is set. +struct NoQsWriter(Box); + +impl AlignmentWriter for NoQsWriter { + fn write_batch( + &mut self, + batch: &[noodles::sam::alignment::record_buf::RecordBuf], + ) -> Result<(), error::Error> { + let mut stripped: Vec<_> = batch.to_vec(); + for record in &mut stripped { + *record.quality_scores_mut() = + noodles::sam::alignment::record_buf::QualityScores::default(); + } + self.0.write_batch(&stripped) + } + + fn finish(&mut self) -> Result<(), error::Error> { + self.0.finish() + } +} + /// Null writer that discards all output (for two-pass mode pass 1) struct NullWriter; @@ -503,7 +528,8 @@ fn run_smartseq( match &cell.read2 { // Single-end: count reads. None => { - let mut reader = crate::io::fastq::FastqReader::open(&cell.read1, cmd)?; + let mut reader = + crate::io::fastq::FastqReader::open(&cell.read1, cmd)?.with_params(params); loop { let batch = reader.read_batch(10_000)?; if batch.is_empty() { @@ -536,7 +562,8 @@ fn run_smartseq( // Paired-end: align both mates as a fragment, count the fragment once // (gene from the union of both mates' overlaps). Some(r2) => { - let mut reader = crate::io::fastq::PairedFastqReader::open(&cell.read1, r2, cmd)?; + let mut reader = crate::io::fastq::PairedFastqReader::open(&cell.read1, r2, cmd)? + .with_params(params); loop { let mut batch = Vec::with_capacity(10_000); while batch.len() < 10_000 { @@ -671,65 +698,78 @@ fn run_single_pass( let out_type = ¶ms.out_sam_type; // Build boxed writer — stdout takes precedence over file output. - let mut writer: Box = match params.out_std { - OutStd::Sam => { - info!("Writing SAM to stdout (--outStd SAM)"); - Box::new(crate::io::sam::SamStdoutWriter::create( - &index.genome, - params, - )?) - } - OutStd::BamUnsorted => { - info!("Writing unsorted BAM to stdout (--outStd BAM_Unsorted)"); - Box::new(crate::io::bam::BamStdoutWriter::create( - &index.genome, - params, - )?) - } - OutStd::BamSortedByCoordinate => { - info!("Writing coordinate-sorted BAM to stdout (--outStd BAM_SortedByCoordinate)"); - Box::new(crate::io::bam::SortedBamStdoutWriter::create( - &index.genome, - params, - )?) - } - OutStd::None => match out_type.format { - OutSamFormat::Sam => { - let output_path = params.output_path("Aligned.out.sam"); - info!("Writing SAM to {}", output_path.display()); - if let Some(parent) = output_path.parent() { - std::fs::create_dir_all(parent)?; - } - Box::new(SamWriter::create(&output_path, &index.genome, params)?) + // `--outSAMmode None` asks for no alignment output at all; everything else + // (SJ.out.tab, Log.final.out, counts) is still produced, so the alignment + // is still run and only the records are dropped. + let mut writer: Box = if params.out_sam_mode == "None" { + info!("--outSAMmode None: no alignment records will be written"); + Box::new(NullWriter) + } else { + match params.out_std { + OutStd::Sam => { + info!("Writing SAM to stdout (--outStd SAM)"); + Box::new(crate::io::sam::SamStdoutWriter::create( + &index.genome, + params, + )?) } - OutSamFormat::Bam => { - let sorted = out_type.sort_order == Some(OutSamSortOrder::SortedByCoordinate); - let output_path = if sorted { - params.output_path("Aligned.sortedByCoord.out.bam") - } else { - params.output_path("Aligned.out.bam") - }; - info!("Writing BAM to {}", output_path.display()); - if let Some(parent) = output_path.parent() { - std::fs::create_dir_all(parent)?; - } - if sorted { - Box::new(SortedBamWriter::create( - &output_path, - &index.genome, - params, - )?) - } else { - Box::new(BamWriter::create(&output_path, &index.genome, params)?) - } + OutStd::BamUnsorted => { + info!("Writing unsorted BAM to stdout (--outStd BAM_Unsorted)"); + Box::new(crate::io::bam::BamStdoutWriter::create( + &index.genome, + params, + )?) } - OutSamFormat::None => { - info!("--outSAMtype None: skipping alignment output (count/quant only)"); - Box::new(NullWriter) + OutStd::BamSortedByCoordinate => { + info!("Writing coordinate-sorted BAM to stdout (--outStd BAM_SortedByCoordinate)"); + Box::new(crate::io::bam::SortedBamStdoutWriter::create( + &index.genome, + params, + )?) } - }, + OutStd::None => match out_type.format { + OutSamFormat::Sam => { + let output_path = params.output_path("Aligned.out.sam"); + info!("Writing SAM to {}", output_path.display()); + if let Some(parent) = output_path.parent() { + std::fs::create_dir_all(parent)?; + } + Box::new(SamWriter::create(&output_path, &index.genome, params)?) + } + OutSamFormat::Bam => { + let sorted = out_type.sort_order == Some(OutSamSortOrder::SortedByCoordinate); + let output_path = if sorted { + params.output_path("Aligned.sortedByCoord.out.bam") + } else { + params.output_path("Aligned.out.bam") + }; + info!("Writing BAM to {}", output_path.display()); + if let Some(parent) = output_path.parent() { + std::fs::create_dir_all(parent)?; + } + if sorted { + Box::new(SortedBamWriter::create( + &output_path, + &index.genome, + params, + )?) + } else { + Box::new(BamWriter::create(&output_path, &index.genome, params)?) + } + } + OutSamFormat::None => { + info!("--outSAMtype None: skipping alignment output (count/quant only)"); + Box::new(NullWriter) + } + }, + } }; + // `--outSAMmode NoQS`: emit records without their quality strings. + if params.out_sam_mode == "NoQS" { + writer = Box::new(NoQsWriter(writer)); + } + // Align reads through the boxed writer. // // Solo runs supply two `--readFilesIn` files (cDNA read + barcode read) but @@ -749,7 +789,8 @@ fn run_single_pass( w.finish()?; } let sj_output_path = params.output_path("SJ.out.tab"); - if !sj_stats.is_empty() { + // `--outSJtype None` suppresses SJ.out.tab entirely. + if params.out_sj_type != "None" && !sj_stats.is_empty() { sj_stats.write_output(&sj_output_path, &index.genome, params)?; } // Per-cell count matrices (raw + filtered), Summary.csv, and the SJ @@ -796,7 +837,8 @@ fn run_single_pass( // 5. Write SJ.out.tab file let sj_output_path = params.output_path("SJ.out.tab"); - if !sj_stats.is_empty() { + // `--outSJtype None` suppresses SJ.out.tab entirely. + if params.out_sj_type != "None" && !sj_stats.is_empty() { info!( "Writing splice junction statistics to {}", sj_output_path.display() @@ -1369,7 +1411,8 @@ fn align_reads_single_end( let read_file = ¶ms.read_files_in[0]; info!("Reading single-end from {}", read_file.display()); - let reader = FastqReader::open(read_file, params.read_files_command.as_deref())?; + let reader = + FastqReader::open(read_file, params.read_files_command.as_deref())?.with_params(params); // Create chimeric output writer if enabled let chimeric_writer = if params.chim_segment_min > 0 && params.chim_out_junctions() { @@ -2708,7 +2751,8 @@ fn align_reads_paired_end( ¶ms.read_files_in[0], ¶ms.read_files_in[1], params.read_files_command.as_deref(), - )?; + )? + .with_params(params); // Create chimeric output writer if enabled let chimeric_writer = if params.chim_segment_min > 0 && params.chim_out_junctions() { diff --git a/src/params/mod.rs b/src/params/mod.rs index 6eaea55..5c3f41a 100644 --- a/src/params/mod.rs +++ b/src/params/mod.rs @@ -1692,6 +1692,100 @@ impl Parameters { )); } + // --readFilesPrefix is prepended to every input read path, so apply it + // here and let every consumer see the final paths. + if !params.read_files_prefix.is_empty() { + let prefix = std::path::Path::new(¶ms.read_files_prefix); + for rf in &mut params.read_files_in { + *rf = prefix.join(&*rf); + } + } + + // Validate --outSAMmode. + if !matches!(params.out_sam_mode.as_str(), "Full" | "NoQS" | "None") { + return Err(command.error( + ErrorKind::InvalidValue, + format!( + "unknown --outSAMmode '{}'; expected Full, NoQS, or None", + params.out_sam_mode + ), + )); + } + // Validate --outSAMorder. Both values are already satisfied: the batch + // pipeline consumes batches in input order, so alignments come out in + // the order their reads went in. + if !matches!( + params.out_sam_order.as_str(), + "Paired" | "PairedKeepInputOrder" + ) { + return Err(command.error( + ErrorKind::InvalidValue, + format!( + "unknown --outSAMorder '{}'; expected Paired or PairedKeepInputOrder", + params.out_sam_order + ), + )); + } + // Validate --outSJtype. + if !matches!(params.out_sj_type.as_str(), "Standard" | "None") { + return Err(command.error( + ErrorKind::InvalidValue, + format!( + "unknown --outSJtype '{}'; expected Standard or None", + params.out_sj_type + ), + )); + } + // Validate --outSJfilterReads. + if !matches!(params.out_sj_filter_reads.as_str(), "All" | "Unique") { + return Err(command.error( + ErrorKind::InvalidValue, + format!( + "unknown --outSJfilterReads '{}'; expected All or Unique", + params.out_sj_filter_reads + ), + )); + } + // Validate --readQualityScoreBase. + if !matches!(params.read_quality_score_base, 0 | 33 | 64) { + return Err(command.error( + ErrorKind::InvalidValue, + format!( + "unsupported --readQualityScoreBase {}; expected 33 or 64", + params.read_quality_score_base + ), + )); + } + // --outSAMfilter: the added-reference modes require inserting + // --genomeFastaFiles references at alignment time, which this aligner + // does not do. Reject them loudly rather than accept and ignore. + for f in ¶ms.out_sam_filter { + if f.as_str() != "None" { + return Err(command.error( + ErrorKind::InvalidValue, + format!( + "--outSAMfilter {f} is not supported: it requires inserting \ + --genomeFastaFiles references at alignment time" + ), + )); + } + } + // --readFilesType: only Fastx is supported. Reading pre-aligned SAM + // input is separate work; refuse rather than silently treating a SAM + // file as FASTQ. + { + let kind = params + .read_files_type + .first() + .map_or("Fastx", String::as_str); + if kind != "Fastx" { + return Err(command.error( + ErrorKind::InvalidValue, + format!("--readFilesType {kind} is not supported; expected Fastx"), + )); + } + } + // ── STARsolo validation ───────────────────────────────────────── if params.run_mode == RunMode::AlignReads && params.solo_enabled() { // CB_UMI_Complex needs one CB position + whitelist per segment. From 6d3651543143bffc4144083bc3f470965d903b1f Mon Sep 17 00:00:00 2001 From: Benjamin Demaille Date: Tue, 28 Jul 2026 21:22:53 +0200 Subject: [PATCH 3/5] test(cli): end-to-end coverage for the new knobs, plus changelog Nine integration tests over the SAM/SJ/read-input knobs: accepted values, rejected values, `--readFilesPrefix` applied to both mates, negative `--outQSconversionAdd` surviving clap's hyphen handling, and the inert limit family parsing at STAR's defaults. They drive `Parameters` directly rather than spawning the binary: every knob here is decided at parse time or at a single output site, so a full alignment run would add minutes without adding coverage. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 25 +++++++ tests/cli_output_parity.rs | 131 +++++++++++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+) create mode 100644 tests/cli_output_parity.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index b6e4893..1927cbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,31 @@ Sections commonly used: Features, Bug fixes, Other changes. ### Features +- **CLI and output parity: SAM/SJ/read-input knobs and the STAR limit + surface** — 31 further STAR 2.7.11b parameters, taking the accepted surface + from 143/200 to 174/200. + + - Implemented: `--outSAMmode` (`Full`/`NoQS`/`None`), `--outSJtype None`, + `--outSJfilterReads Unique`, `--outSAMheaderHD`, `--outSAMheaderPG`, + `--outSAMheaderCommentFile`, `--readFilesPrefix`, `--readNameSeparator`, + `--readQualityScoreBase`, `--outQSconversionAdd`. + - Refused loudly rather than accepted and ignored: + `--outSAMfilter KeepOnlyAddedReferences` / `KeepAllAddedReferences` and + `--readFilesType SAM`, both of which need machinery this aligner does not + have. + - Accepted and documented as output-neutral: the `--limit*` family, + `--outTmpDir`, `--outTmpKeep`, `--runDirPerm`, `--genomeFileSizes`, + `--outBAMsorting*`, `--readMatesLengthsIn`. + + `tests/parameter_surface.rs` now checks every STAR 2.7.11b parameter name + against the CLI, so the coverage figure is machine-checked and surface drift + fails a test. + +### Bug fixes + +- Read names are cut at `--readNameSeparator` (default `/`), as STAR does. A + read named `foo/1` was previously emitted as `foo/1` where STAR emits `foo`. + - **STARsolo single-cell quantification (`--soloType`)** — the 10x Chromium / plate-based count-matrix pipeline, ported from STAR and verified against real STARsolo (#90). diff --git a/tests/cli_output_parity.rs b/tests/cli_output_parity.rs new file mode 100644 index 0000000..96659e5 --- /dev/null +++ b/tests/cli_output_parity.rs @@ -0,0 +1,131 @@ +//! End-to-end checks for the SAM/SJ/read-input knobs. +//! +//! These drive `Parameters` directly rather than spawning the binary: the +//! knobs under test are all decided at parse time or at a single output site, +//! so a full alignment run would add minutes without adding coverage. + +use rustar_aligner::params::Parameters; + +fn parse(args: &[&str]) -> Result { + let mut full = vec!["rustar-aligner"]; + full.extend_from_slice(args); + Parameters::try_parse_from(&full) +} + +fn with_reads(args: &[&str]) -> Result { + let mut full = vec!["--readFilesIn", "reads.fq"]; + full.extend_from_slice(args); + parse(&full) +} + +#[test] +fn out_sam_mode_accepts_stars_three_values_and_rejects_others() { + for value in ["Full", "NoQS", "None"] { + assert!( + with_reads(&["--outSAMmode", value]).is_ok(), + "--outSAMmode {value} should parse" + ); + } + assert!(with_reads(&["--outSAMmode", "Quiet"]).is_err()); +} + +#[test] +fn out_sj_knobs_accept_stars_values_and_reject_others() { + assert_eq!(with_reads(&[]).unwrap().out_sj_type, "Standard"); + assert!(with_reads(&["--outSJtype", "None"]).is_ok()); + assert!(with_reads(&["--outSJtype", "Compact"]).is_err()); + + assert_eq!(with_reads(&[]).unwrap().out_sj_filter_reads, "All"); + assert!(with_reads(&["--outSJfilterReads", "Unique"]).is_ok()); + assert!(with_reads(&["--outSJfilterReads", "Best"]).is_err()); +} + +#[test] +fn read_files_prefix_is_applied_to_every_input_path() { + let p = with_reads(&["--readFilesPrefix", "/data/run7/"]).unwrap(); + assert_eq!(p.read_files_in[0].to_str().unwrap(), "/data/run7/reads.fq"); + + // Paired input: both mates get the prefix. + let p = parse(&[ + "--readFilesIn", + "r1.fq", + "r2.fq", + "--readFilesPrefix", + "/data/run7/", + ]) + .unwrap(); + assert_eq!(p.read_files_in[0].to_str().unwrap(), "/data/run7/r1.fq"); + assert_eq!(p.read_files_in[1].to_str().unwrap(), "/data/run7/r2.fq"); + + // Absent by default. + let p = with_reads(&[]).unwrap(); + assert_eq!(p.read_files_in[0].to_str().unwrap(), "reads.fq"); +} + +#[test] +fn read_name_separator_defaults_to_stars_slash() { + let p = with_reads(&[]).unwrap(); + assert_eq!(p.read_name_separator, vec!["/".to_string()]); + + let p = with_reads(&["--readNameSeparator", "-"]).unwrap(); + assert_eq!(p.read_name_separator, vec!["-".to_string()]); +} + +#[test] +fn read_quality_score_base_is_restricted_to_the_two_real_encodings() { + assert_eq!(with_reads(&[]).unwrap().read_quality_score_base, 33); + assert!(with_reads(&["--readQualityScoreBase", "64"]).is_ok()); + assert!(with_reads(&["--readQualityScoreBase", "0"]).is_ok()); + assert!(with_reads(&["--readQualityScoreBase", "40"]).is_err()); +} + +#[test] +fn out_qs_conversion_add_takes_negative_values() { + // -31 is the Phred+64 to Phred+33 conversion, and needs to survive clap's + // hyphen handling. + let p = with_reads(&["--outQSconversionAdd", "-31"]).unwrap(); + assert_eq!(p.out_qs_conversion_add, -31); +} + +#[test] +fn unsupported_modes_are_refused_rather_than_ignored() { + // Both need machinery this aligner does not have. Accepting and ignoring + // them would silently produce output the user did not ask for. + assert!(with_reads(&["--outSAMfilter", "KeepOnlyAddedReferences"]).is_err()); + assert!(with_reads(&["--outSAMfilter", "KeepAllAddedReferences"]).is_err()); + assert!(with_reads(&["--readFilesType", "SAM", "SE"]).is_err()); + + // The defaults, and the supported input type, still parse. + assert!(with_reads(&["--outSAMfilter", "None"]).is_ok()); + assert!(with_reads(&["--readFilesType", "Fastx"]).is_ok()); +} + +#[test] +fn out_sam_order_accepts_both_values() { + // `run_batch_pipeline` consumes batches in input order, so + // PairedKeepInputOrder is already satisfied and both values are honest. + assert!(with_reads(&["--outSAMorder", "Paired"]).is_ok()); + assert!(with_reads(&["--outSAMorder", "PairedKeepInputOrder"]).is_ok()); + assert!(with_reads(&["--outSAMorder", "Unsorted"]).is_err()); +} + +#[test] +fn inert_limit_knobs_parse_with_stars_defaults() { + let p = with_reads(&[]).unwrap(); + assert_eq!(p.limit_out_sam_one_read_bytes, 100_000); + assert_eq!(p.limit_out_sj_collapsed, 1_000_000); + assert_eq!(p.limit_out_sj_one_read, 1_000); + assert_eq!(p.limit_sjdb_insert_nsj, 1_000_000); + assert_eq!(p.limit_nreads_soft, -1); + assert_eq!(p.limit_io_buffer_size, vec![30_000_000, 50_000_000]); + assert_eq!(p.run_dir_perm, "User_RWX"); + assert_eq!(p.out_tmp_dir, "-"); + assert_eq!(p.out_tmp_keep, "None"); + assert_eq!(p.out_bam_sorting_bins_n, 50); + assert_eq!(p.out_bam_sorting_thread_n, 0); + + // And they accept non-default values without complaint, since STAR does. + assert!(with_reads(&["--limitOutSJcollapsed", "2000000"]).is_ok()); + assert!(with_reads(&["--limitNreadsSoft", "-1"]).is_ok()); + assert!(with_reads(&["--outBAMsortingThreadN", "8"]).is_ok()); +} From 4ce7634686904c95d6ee2058b4e8334d592dc634 Mon Sep 17 00:00:00 2001 From: Benjamin Demaille Date: Tue, 28 Jul 2026 22:30:37 +0200 Subject: [PATCH 4/5] chore: rebase onto #145, dropping the outSAMorder declaration it now owns #145 landed `--outSAMorder` and `--alignEndsType` while this branch was open. The duplicate `--outSAMorder` declaration and its validation are removed, along with the test that covered them, and `alignEndsType` moves out of `NOT_YET_ACCEPTED` since it is now implemented upstream. The parameter-surface test needed no other change, which is the point of it: it reports the new number by itself. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 3 +-- src/params/mod.rs | 20 -------------------- tests/cli_output_parity.rs | 9 --------- tests/parameter_surface.rs | 1 - 4 files changed, 1 insertion(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1927cbb..d80b3d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,8 +14,7 @@ Sections commonly used: Features, Bug fixes, Other changes. ### Features - **CLI and output parity: SAM/SJ/read-input knobs and the STAR limit - surface** — 31 further STAR 2.7.11b parameters, taking the accepted surface - from 143/200 to 174/200. + surface** — 30 further STAR 2.7.11b parameters. (`--outSAMorder` came from #145.) - Implemented: `--outSAMmode` (`Full`/`NoQS`/`None`), `--outSJtype None`, `--outSJfilterReads Unique`, `--outSAMheaderHD`, `--outSAMheaderPG`, diff --git a/src/params/mod.rs b/src/params/mod.rs index 5c3f41a..bc909f6 100644 --- a/src/params/mod.rs +++ b/src/params/mod.rs @@ -703,11 +703,6 @@ pub struct Parameters { #[arg(long = "outSAMmode", default_value = "Full")] pub out_sam_mode: String, - /// Order of alignments in the output: `Paired` (default) or - /// `PairedKeepInputOrder`. - #[arg(long = "outSAMorder", default_value = "Paired")] - pub out_sam_order: String, - /// Post-alignment record filter. Only the default (no filtering) is /// supported; the added-reference modes need align-time reference /// insertion, which this aligner does not do. @@ -1711,21 +1706,6 @@ impl Parameters { ), )); } - // Validate --outSAMorder. Both values are already satisfied: the batch - // pipeline consumes batches in input order, so alignments come out in - // the order their reads went in. - if !matches!( - params.out_sam_order.as_str(), - "Paired" | "PairedKeepInputOrder" - ) { - return Err(command.error( - ErrorKind::InvalidValue, - format!( - "unknown --outSAMorder '{}'; expected Paired or PairedKeepInputOrder", - params.out_sam_order - ), - )); - } // Validate --outSJtype. if !matches!(params.out_sj_type.as_str(), "Standard" | "None") { return Err(command.error( diff --git a/tests/cli_output_parity.rs b/tests/cli_output_parity.rs index 96659e5..25f62da 100644 --- a/tests/cli_output_parity.rs +++ b/tests/cli_output_parity.rs @@ -100,15 +100,6 @@ fn unsupported_modes_are_refused_rather_than_ignored() { assert!(with_reads(&["--readFilesType", "Fastx"]).is_ok()); } -#[test] -fn out_sam_order_accepts_both_values() { - // `run_batch_pipeline` consumes batches in input order, so - // PairedKeepInputOrder is already satisfied and both values are honest. - assert!(with_reads(&["--outSAMorder", "Paired"]).is_ok()); - assert!(with_reads(&["--outSAMorder", "PairedKeepInputOrder"]).is_ok()); - assert!(with_reads(&["--outSAMorder", "Unsorted"]).is_err()); -} - #[test] fn inert_limit_knobs_parse_with_stars_defaults() { let p = with_reads(&[]).unwrap(); diff --git a/tests/parameter_surface.rs b/tests/parameter_surface.rs index ff579c1..7516862 100644 --- a/tests/parameter_surface.rs +++ b/tests/parameter_surface.rs @@ -89,7 +89,6 @@ const NOT_YET_ACCEPTED: &[&str] = &[ // Aligner core (annotated-junction stitching, alignEndsType, in-recursion // length penalty). "alignEndsProtrude", - "alignEndsType", "alignInsertionFlush", "alignSoftClipAtReferenceEnds", "alignTranscriptsPerReadNmax", From 92e845962a76b82161898d58e6908c63e7c9027e Mon Sep 17 00:00:00 2001 From: Benjamin Demaille Date: Wed, 29 Jul 2026 22:38:32 +0200 Subject: [PATCH 5/5] test(params): stop excluding three STAR parameters from the surface fixture The fixture's own header said it was STAR's parametersDefault names "minus the three non-user meta-params parametersFiles, sysShell, versionGenome". All three are settable on STAR's command line like any other parameter, so removing them did not describe a narrower surface, it hid three names rustar-aligner does not accept and shrank the denominator the coverage figure is quoted against. The fixture is now every column-1 name of parametersDefault, 203 of them, and the three sit in NOT_YET_ACCEPTED where the honest gap is recorded. Coverage reads 175/203 rather than 175/200. clap rejecting them is the right behaviour meanwhile, and worth keeping deliberately: silently accepting --parametersFiles would discard every parameter in the file the user passed. --- tests/data/star_2.7.11b_params.txt | 18 +++++++++++++----- tests/parameter_surface.rs | 11 +++++++++-- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/tests/data/star_2.7.11b_params.txt b/tests/data/star_2.7.11b_params.txt index 66edbae..f961db6 100644 --- a/tests/data/star_2.7.11b_params.txt +++ b/tests/data/star_2.7.11b_params.txt @@ -1,8 +1,13 @@ -# STAR 2.7.11b user-facing parameter names (the public CLI surface, not implementation). -# Derived from the column-1 names of STAR 2.7.11b's parametersDefault, minus the three -# non-user meta-params parametersFiles, sysShell, versionGenome. This is a NAME list only -# (the public interface), never STAR source; it locks the CLI parameter-surface parity via -# tests/parameter_surface.rs. 200 names. +# STAR 2.7.11b user-facing parameter names: the public CLI surface, not implementation. +# Every column-1 name of STAR 2.7.11b's parametersDefault, with none removed. This is a +# NAME list only (the public interface), never STAR source; it locks CLI parameter-surface +# parity via tests/parameter_surface.rs. +# +# An earlier revision dropped parametersFiles, sysShell and versionGenome as +# "non-user meta-params". They are settable on STAR's command line like any other, so +# leaving them out inflated the coverage figure by hiding three names rustar-aligner does +# not accept. They are back, and listed in NOT_YET_ACCEPTED where they belong. +# 203 names. alignEndsProtrude alignEndsType alignInsertionFlush @@ -118,6 +123,7 @@ outWigNorm outWigReferencesPrefix outWigStrand outWigType +parametersFiles peOverlapMMp peOverlapNbasesMin quantMode @@ -193,9 +199,11 @@ soloUMIfiltering soloUMIlen soloUMIposition soloUMIstart +sysShell twopass1readsN twopassMode varVCFfile +versionGenome waspOutputMode winAnchorDistNbins winAnchorMultimapNmax diff --git a/tests/parameter_surface.rs b/tests/parameter_surface.rs index 7516862..8b573e4 100644 --- a/tests/parameter_surface.rs +++ b/tests/parameter_surface.rs @@ -1,7 +1,7 @@ //! Locks the CLI parameter surface against STAR 2.7.11b. //! -//! `tests/data/star_2.7.11b_params.txt` lists every user-facing parameter name -//! STAR 2.7.11b accepts. This test asserts that clap recognises each one, so +//! `tests/data/star_2.7.11b_params.txt` lists every column-1 name of STAR +//! 2.7.11b's `parametersDefault`, with none removed. This test asserts that clap recognises each one, so //! the coverage figure is a machine-checked number rather than a claim, and so //! surface drift shows up the moment it happens rather than in a bug report. //! @@ -86,6 +86,13 @@ const ACCEPTED_BUT_INERT: &[(&str, &str)] = &[ /// Adding a name here must always be a deliberate act. Removing one is what /// progress looks like. const NOT_YET_ACCEPTED: &[&str] = &[ + // STAR meta-parameters, none of them accepted here. clap rejects them, so + // a user who passes one is told rather than quietly ignored, which is the + // behaviour these three need most: silently dropping `--parametersFiles` + // would discard every parameter in that file. + "parametersFiles", + "sysShell", + "versionGenome", // Aligner core (annotated-junction stitching, alignEndsType, in-recursion // length penalty). "alignEndsProtrude",