lazear / lazear/sage

mzML parser ignores `referenceableParamGroupRef`

Open
#232 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
309
Forks
66
PR merge metrics
No merged PRs in 30d

Description

Howdy!
IDK if this is a feature request/pitch or a bug report...
in essence a part of the mzML spec is not implemented and skips scans from some converters (I got it from a sciex converter) because it does not refenrence the values in a "referenceableParamGroupRef".

LMK if you want a PR for this guy ...

> Completely ai-generated and super verbose bug report

# mzML parser ignores `referenceableParamGroupRef`, silently dropping spectra whose `ms level` is defined in a shared param group

## Summary

Sage's mzML reader does not resolve `referenceableParamGroupRef` elements. Any
`cvParam` that a valid mzML file factors out into a ``
and attaches to a spectrum by reference is invisible to Sage. When the shared
group holds the `ms level` term (`MS:1000511`) — a common and spec-legal way to
write it — the affected spectra are parsed with no MS level, default to level
`0`, and are silently excluded from the search. No warning or error is emitted;
the run completes and simply reports far fewer (often zero) identifications.

## Environment

- Sage `v0.15.0-beta.2`
- Reader: `crates/sage-cloudpath/src/mzml.rs`

## This is valid mzML

`referenceableParamGroup` is part of the PSI mzML 1.1.0 schema
(`mzML1.1.0.xsd`). `` defines named groups of
`cvParam`/`userParam` at the top of the run; any element built on
`ParamGroupType` — including ``, ``, and `` —
may then pull those params in with a ``.
The referenced params are semantically identical to writing the same `cvParam`
elements inline. Factoring shared terms (polarity, spectrum type, and MS level)
into a group is an explicitly supported feature, used to keep files compact when
thousands of spectra share the same descriptors.

Minimal legal fragment:

```xml




...



...

```

## Root cause

The reader is a flat state machine over start/end elements
(`crates/sage-cloudpath/src/mzml.rs`). It enters `State::Spectrum` on
`` and reads MS level only from a `cvParam` **directly** nested in that
state:

```rust
// mzml.rs:194
(Some(State::Spectrum), b"cvParam") => {
let accession = extract!(ev, b"accession");
match accession.as_ref() {
MS_LEVEL => {
let level = extract_value!(ev);
...
spectrum.ms_level = level; // mzml.rs:205
}
...
}
}
```

There is no handling anywhere for `referenceableParamGroupList`,
`referenceableParamGroup`, or `referenceableParamGroupRef` — the strings do not
appear in the crate. A `` inside a spectrum is an
unrecognized start element and is skipped, and the group's `cvParam`s (which
live earlier in the file, outside any ``) are never associated with
the spectrum.

Consequently `spectrum.ms_level` keeps its default value of `0`. Downstream that
value matches neither the MS1 nor the MS2 path
(`crates/sage/src/spectrum.rs` dispatches on `ms_level == 1` / `ms_level == 2`),
so the spectrum contributes nothing. If a level filter is configured, the binary
arrays are skipped outright (`mzml.rs:286`). Either way the spectrum is dropped
with no diagnostic.

## Failing test

Drop-in for the `test` module in `crates/sage-cloudpath/src/mzml.rs`. It parses
a single MS2 spectrum whose `ms level` is supplied through a
`` and asserts the level is read as `2`. No FASTA or
search is involved — it exercises the parser directly.

```rust
#[tokio::test]
async fn parse_referenceable_param_group_ms_level() -> Result<(), MzMLError> {
let s = r#"


















AAAAAAAALkAAAAAAAAA0QAAAAAAAADlA





AAAAAAAAWUAAAAAAAABZQAAAAAAAAFlA






"#;

let mut spectra = MzMLReader::with_file_id(0).parse(s.as_bytes()).await?;
assert_eq!(spectra.len(), 1);
let s = spectra.pop().unwrap();

assert_eq!(s.id, "referenced=1");
assert_eq!(
s.ms_level, 2,
"ms level from referenceableParamGroupRef was not resolved"
);
Ok(())
}
```

Actual output on `v0.15.0-beta.2`:

```
running 1 test
test mzml::test::parse_referenceable_param_group_ms_level ... FAILED

---- mzml::test::parse_referenceable_param_group_ms_level stdout ----
thread '...' panicked at crates/sage-cloudpath/src/mzml.rs:
assertion `left == right` failed: ms level from referenceableParamGroupRef was not resolved
left: 0
right: 2

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 22 filtered out
```

The spectrum parses (it is not rejected), but `ms_level` comes back `0` instead
of `2`: the group reference was skipped. The same spectrum with the three
`cvParam`s written inline parses as level `2` and the test passes — which
isolates `referenceableParamGroupRef` resolution as the sole difference. In a
real file every referenced MS2 spectrum lands at level `0` and is excluded from
the search.

## Impact

- The file gets read but 0 spectra pass through.

## Suggested fix

Resolve references at parse time:

1. Parse `` into a map `id -> Vec` before
(or during) the spectrum pass.
2. On `` inside a spectrum/scan/
binaryDataArray, replay the group's stored `cvParam`s through the same
accession-matching logic already used for inline `cvParam`s.

Contributor guide

Open the contributing guide

Research direction

Start in crates/sage-cloudpath/src/mzml.rs by reading the parser state handling for spectra and cvParam elements, then add the provided parse_referenceable_param_group_ms_level test to the test module. Verify that the test reads ms_level as 2 and that referenced parameters are handled for the parser contexts described in the issue.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
bioinformatics
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.