Ptm stoich

[pcruzparri]

Adds occupancy calculation to MM using peptide intensities from FlashLFQ. Backend calculation currently implemented in the MzLib.MzLibUtil.PositionFrequencyAnalysis class. In MM, the updating of the ProteinGroup.ModInfo field with intensity-based stoichiometry is done in the PostSearchAnalysis.QuantificationAnalysis() method, and the ProteinGroup.ToString() takes the ModInfo data to produce the occupancy string.

Description

This PR introduces PTM stoichiometry/occupancy quantification to MetaMorpheus, refactors ProteinGroup to inherit from mzLib's BioPolymerGroup, and adds several quality-of-life improvements and bug fixes.

Key Changes

1. PTM Occupancy by Spectral Count and Intensity
Protein group TSV output now includes four new families of dynamic columns per sample group:

• SpectralCount_<sample> — number of PSMs observing the protein
• Intensity_<sample> — FlashLFQ intensity per sample
• CountOccupancy_<sample> — modified spectral count / total spectral count
• IntensityOccupancy_<sample> — modified intensity / total intensity
  These columns are generated via the new PopulateSampleGroupResults() method on BioPolymerGroup. Columns only appear when quantification state is available upstream, preserving backward compatibility for workflows that return early from quantification.

2. ProteinGroup → BioPolymerGroup Inheritance
ProteinGroup now extends Omics.BioPolymerGroup.BioPolymerGroup from mzLib, delegating quantification, sample grouping, and occupancy logic to the base class. Backward-compatible alias properties (Proteins, AllPeptides, ProteinGroupName, IntensitiesByFile, etc.) map to base class properties so existing consumers are unaffected.

3. ScanMetadata Record
Scan metadata (scan number, retention time, precursor charge/mass/intensity, ion mobility, etc.) is now captured in a lightweight immutable ScanMetadata record on Ms2ScanWithSpecificMass and SpectralMatch. This allows heavyweight scan objects to be released from memory after scoring while preserving metadata access via pass-through properties.

4. SpectralMatch → ISpectralMatch Interface
SpectralMatch now implements ISpectralMatch from mzLib, with explicit interface members for OneBasedScanNumber, Intensities, GetIdentifiedBioPolymersWithSetMods(), CompareTo(ISpectralMatch?), and Equals(ISpectralMatch?).

5. ThreadSafeObservableCollection Improvements
Updated to facilitate safe iteration through the collection without worrying about threading issues, with comprehensive test coverage.

6. Calling Code Adapted to New Property Semantics
IntensitiesByFile changed from a simple auto-property (getter returned the backing reference) to an adapter property whose getter returns a new copy via .ToDictionary(). Calling code in PostSearchAnalysisTask and SilacConversions was updated from the old mutate-the-getter pattern (.Add() on the returned dictionary) to the build-locally-then-assign pattern.

7. Bug Fixes

• Fixed variant description access (variant.Description instead of variant.VariantCallFormatDataString.Description) with null-safety guard
• Fixed possibleVariantPsms ordering result being discarded (now assigned back)
• Fixed early return from quantification when experimental design is unavailable, while still writing count-based occupancy columns
• Fixed test isolation issue where stale ExperimentalDesign.tsv in shared TestData/ directory affected other tests

[trishorts]

Automated code review - 4 finding(s) at severity >= Major, posted as inline comments.

[nbollis]

Review Summary

Approved findings from the dashboard were revalidated against the current PR head after a freshness sync. The head commit changed since initial analysis, but both approved findings still resolved to valid inline anchors on the updated patch.

Major (2)

• MetaMorpheus/EngineLayer/ProteinParsimony/ProteinGroup.cs:99 Alias collection getters now return detached copies instead of live mutable collections, creating a silent behavioral break for callers that mutate getter results.
• MetaMorpheus/EngineLayer/ProteinParsimony/ProteinGroup.cs:415 ProteinGroup's custom coverage calculation no longer keeps the inherited BioPolymerGroup.CoverageResult contract in sync, so inherited callers can observe coverage that diverges from MetaMorpheus output.

Grouping: 2 approved findings -> 2 inline comments | 0 summary-only.

[trishorts]

Automated code review - 1 finding(s) at severity >= Major, posted as inline comments.

[trishorts]

Approving. I re-reviewed against the current head (ae97ad2) — not just the reply threads — and every Critical/Major finding from this round is resolved or acknowledged-and-deferred with a test:

• Variant NRE — reverted to master's VariantCallFormatDataString.Description; the incomplete guard is gone. ✅
• .OfType<SpectralMatch>() dropping PSMs — now a single Cast<SpectralMatch>().ToList() reused for Score/QValue/PEP, so all three are computed over the same population and an unexpected type throws loudly. ✅
• ThreadSafeObservableCollection deadlock (Critical) + PropertyChanged marshalling — whole (out-of-scope) class reverted out of the PR. ✅
• Alias getters silently detached — now IReadOnly with the snapshot semantics documented. ✅
• Glyco PopulateSampleGroupResults on null LFQ — glyco now always assigns IntensitiesByFile (zeros when no LFQ) and populates uniformly, matching the search path. ✅

Nice, responsive turnaround — the two genuinely risky/out-of-scope items (the deadlock and the variant patch) were cleanly reverted rather than papered over.

Follow-up notes (non-blocking — for the deferred BioPolymerGroup migration PR)

Two of the findings were reasonably deferred with tests. Recording the residual debt so it isn't lost in the reply threads:

1. Method-hiding is a trap for the very migration this defers to. GetTabSeparatedHeader() and CalculateSequenceCoverage() are now new-hidden on a type that is-a BioPolymerGroup. It's safe today only because every caller uses the concrete type — I confirmed WriteProteinGroupsToTsv (PostSearchAnalysisTask.cs:1554) holds List<ProteinGroup> and calls .First().GetTabSeparatedHeader(), so the MM override wins. But the follow-up that moves logic onto BioPolymerGroup must resolve this together with the CoverageResult contract finding (#3443992211) — the first base-typed caller would otherwise silently get empty columns / diverging coverage.

2. Header/row column uniformity is enforced by producer discipline, not structure. The header is built from one representative group while each row re-derives its own columns from its own SampleGroupResults/HasIntensityData. It stays aligned only because the search and glyco paths now both assign IntensitiesByFile for every group (zeros for unmeasured), and TestMultipleProteinGroupsHeaderAndRowsHaveSameColumnCount locks that case. It remains latent TSV-corruption risk if any future path populates SampleGroupResults non-uniformly. The single-shared-schema fix (drive header + rows from one authoritative column shape) is the real resolution and should be the deferred PR — please track it as an issue, not just a reply.

3. Trivial perf, optional: the IntensitiesByFile getter allocates a fresh dictionary on every call, and around ProteinGroup.cs:650 it's called twice to read a single value (null-check then GetValueOrDefault). Cache in a local. Not worth holding anything for.

None of these block the merge.

[nbollis]

Careful. We already have a carefully orchestrated compare method for spectral matches, we should unify them

[nbollis]

There may be issues here with the spectral matches being of the type GlycoSpectralMatch. A cast or something may be more appropriate here, unsure, but please verify this.