Phred encoding is inferred from lowest quality char even for BAM/SAM input, where it is knowable

[FelixKrueger]

Summary

phred::detect infers the quality encoding from the lowest observed quality character. For FASTQ that is the only option available and matches Java FastQC 0.12.1, so no complaint there. For BAM/SAM input it is guessing at something the format already specifies, and it guesses wrong on any dataset whose minimum Phred score is ≥ 31.

BAM QUAL stores raw Phred values (0–93) by spec — there is no ASCII offset to infer. Once the reader converts to ASCII by adding 33, a dataset with no base below Q31 leaves no byte below 64, so detect selects Illumina 1.5 and subtracts 64. Every reported score then comes out 31 too low.

Reproduction

100 reads, 34 bp, uniform Phred+33 'I' (Q40), converted to unaligned BAM and analysed:

BAM QUAL raw bytes : 40 throughout (correct)
fastqc_data.txt : Encoding Illumina 1.5
 base 1 mean 9.0 (true value: 40)

9.0 = 40 − 31. The BAM is spec-correct; only the report is wrong.

Where

src/utils/phred.rs:22

pub fn detect(lowest_char: u8) -> Result<PhredEncoding, String> {
 if lowest_char < 33 { ... }
 else if lowest_char < 64 { /* Sanger / Illumina 1.9 */ }
 ...
 else if lowest_char <= 126 { /* Illumina 1.5 */ }

The heuristic is sound for FASTQ. The issue is that it is reached for BAM/SAM too, where lowest_char carries no information about the encoding — it is an artefact of the reader's own +33 conversion.

I could not find a way to bypass it. FastQCConfig::sequence_format selects the sequence format (fastq/bam/sam), not the quality encoding, and there is no encoding override.

Suggested fix

For BAM/SAM sources, skip detection and use Sanger unconditionally — the reader has already normalised to Phred+33, so that is correct by construction. Either:

1. Short-circuit detect when the input was BAM/SAM, or
2. Expose an encoding override on FastQCConfig, letting callers assert it.

(1) seems preferable: it needs no caller change and there is no case where guessing beats knowing.

Why this surfaced

Found while fixing a --phred64 bug in Trim Galore (FelixKrueger/TrimGalore#358), which uses fastqc-rust for its --fastqc reports including on --output-format ubam output.

Worth noting the misdetection is not BAM-specific in origin — the same uniform-Q40 input misdetects through the FASTQ path as well. But FASTQ genuinely is ambiguous, so there the heuristic is the right answer to an unanswerable question; for BAM it answers a question that has a known answer. Real-world FASTQ almost always contains some low-quality base, which is why this rarely bites in practice.

Happy to send a PR if you'd like, and to defer entirely on which of the two approaches fits the crate better.