Skip to content

feat: ProForma 2.0 reader/writer and Layer-2 modification converter#1072

Open
trishorts wants to merge 20 commits into
smith-chem-wisc:masterfrom
trishorts:feature/proforma-reader-writer
Open

feat: ProForma 2.0 reader/writer and Layer-2 modification converter#1072
trishorts wants to merge 20 commits into
smith-chem-wisc:masterfrom
trishorts:feature/proforma-reader-writer

Conversation

@trishorts

Copy link
Copy Markdown
Contributor

Overview

This PR adds HUPO-PSI ProForma 2.0 reading and writing to mzLib. Layer 1 (string ↔ ProFormaTerm) is delegated to the TopDownProteomics SDK — already a production dependency of mzLib — through a thin Readers/ProForma facade (ProFormaReader, ProFormaWriter), so callers depend on stable mzLib entry points rather than the SDK directly. Layer 2 (ProFormaTermIBioPolymerWithSetMods) is new work: ProFormaConverter maps per-residue and N-/C-terminal modifications to and from mzLib Modification objects, mirroring the existing GetModificationDictionaryFromFullSequence/DetermineFullSequence idiom (N-term = 1, residue = index + 2, C-term = length + 2). Resolution is motif-aware for interior residues and LocationRestriction-aware at termini, handling both names ([Oxidation]) and ontology accessions ([UNIMOD:35]). A one-call IBioPolymerWithSetMods.ToProFormaString() bridge plus a ProForma psmtsv column (written and re-read) complete the round-trip. Unsupported constructs fail loud.

What's included

  • Readers/ProForma/ProFormaReader.Read, ProFormaWriter.Write (wrap the SDK parser/writer); ProFormaConverter (Layer-2, both directions); ProFormaExtensions.ToProFormaString()/ToProFormaTerm().
  • psmtsv columnSpectrumMatchFromTsvHeader.ProForma; SpectrumMatchFromTsv.ProForma parsed via ParseHeader + GetOptionalValue (optional — pre-ProForma files still read).
  • Explicit TopDownProteomics PackageReference added to Readers.csproj (already transitive via UsefulProteomicsDatabases).

Design notes for reviewers

  • Wraps the SDK rather than reimplementing a parser we already ship — least code, reuses a tested implementation.
  • The ProForma facades intentionally do not implement IResultFile: ProForma is an in-memory string codec, not a file-on-disk with an extension. Documented, not an oversight.
  • Out of Layer-2 scope (fails loud with MzLibException): crosslinks/tag groups, branches, labile mods, ambiguity groups, ranges, sequence ambiguity, global isotope mods.
  • Known SDK gaps, each pinned by a test so a future SDK release is detected: (1) writer throws on a range with multiple mods (spec §4.5); (2) v1.0 [SOURCE]+ default-source prefix not parsed (removed in v2.0; intentionally unsupported); (3) charge /z, chimeric +, multi-chain // need a multi-term facade (planned).

Testing

120 ProForma tests (corpus of 123 manuscript examples; Layer-1 round-trip idempotence; Layer-2 round-trips incl. real bundled Unimod validation). Full FileReadingTests suite green (716). Baseline unchanged.

🤖 Generated with Claude Code

trishorts and others added 10 commits May 24, 2026 09:23
Adds the authoritative ProForma test set (123 strings: 26 from the v1.0
paper, 89 valid + 8 counter-examples from the v2.0 spec) as a TSV plus a
CorpusLoaderTests fixture that guards its completeness and integrity. This
is the foundation the reader/writer will be developed against, test-first.

Test data loads via TestContext.CurrentContext.TestDirectory, matching the
existing FileReadingTests convention.

Phase 0: setup + corpus.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds ProFormaReader.Read and ProFormaWriter.Write — thin mzLib entry points
that delegate Layer-1 parse/write to the TopDownProteomics reference parser,
so callers depend on mzLib types rather than the SDK directly. Includes a
shared ProFormaTestCorpus loader and base-level round-trip tests asserting
canonical idempotence of write(parse(s)).

Surfaces the first SDK coverage gap: ProFormaWriter throws on a range bearing
multiple modifications (spec 4.5); parsing succeeds, only writing fails. The
case is pinned by KnownGap_MultiModRange_ParsesButSdkWriterThrows so a future
SDK fix is detected.

Phase 1: Readers/ProForma facade.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Widens the round-trip sweep to every valid corpus example. The wrapped SDK
round-trips all single-proteoform-term forms (99/110): single-chain
crosslinks, disulfides, glycans, formulas/isotopes, ranges, localization
groups, scored sites, unknown position, global mods, termini, and the common
v1.0 syntaxes.

Three gaps are pinned with dedicated tests so each is tracked:
- v1.0 default-source prefix [SOURCE]+SEQ — intentionally unsupported (v2.0
  removed it).
- charge /z, chimeric +, multi-chain // (13 rows) — valid v2.0 features that
  live above a single term; planned facade work.
- writer throws on a multi-mod range (gap #1, pinned earlier).

106 ProForma tests green.

Phase 2-3: Layer-1 coverage map.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds ProFormaConverter mapping between a ProFormaTerm and mzLib's
(base sequence + AllModsOneIsNterminus) representation that
IBioPolymerWithSetMods consumes, mirroring the index convention and lookup
idiom of GetModificationDictionaryFromFullSequence/DetermineFullSequence
(N-terminus = 1, residue r = r + 2, C-terminus = length + 2).

This first slice resolves per-residue Name descriptors via the
"{name} on {residue}" IdWithMotif key and round-trips them back through the
writer. A guard throws MzLibException on features outside the Layer-2 subset
(tag groups/crosslinks, labile, unlocalized, global mods, ranges, sequence
ambiguity) and on modifications absent from the supplied database, so callers
never silently drop data.

Phase 10: Layer-2 converter (slice 1).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extends ProFormaConverter to map Identifier descriptors (e.g. [UNIMOD:35],
[MOD:00719]) to mzLib Modifications via a DatabaseReference accession index,
and to emit accession descriptors on the inverse path when a modification
carries a recognized ontology reference (else a Name).

Resolution is motif-aware: a single accession such as UNIMOD:21 maps to
several modifications differing only by target residue, so the residue at the
tagged position disambiguates. Alias maps bridge mzLib's DatabaseReference
keys and ProForma prefixes (Unimod<->UNIMOD, PSI-MOD<->MOD, RESID<->RESID).

Phase 10: Layer-2 converter (slice 2, accessions).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Makes ProFormaConverter resolution terminus-aware: interior residues match on
target motif, while N-/C-terminal descriptors match on LocationRestriction
(N-terminal./Peptide N-terminal. and the C-terminal equivalents). The Name
path tries the usual "{name} on X" terminal id first, then any same-named
terminus-compatible modification.

Round-trips terminal mods by both name ([Acetyl]-...-[Amidation]) and
accession ([UNIMOD:214]-...), placing them at the AllModsOneIsNterminus
terminal keys (1 and length + 2).

Phase 10: Layer-2 converter (slice 3, terminal mods).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a fixture that loads the bundled Unimod database (~2700 modifications)
via Loaders.LoadUnimod and exercises ProFormaConverter on real data rather
than hand-built mods: resolves names (Oxidation, Phospho) and accessions
(UNIMOD:35, UNIMOD:21), selects the correct motif amid real accession
ambiguity (UNIMOD:21 maps to Phospho on S/T/Y), and round-trips.

Confirms the per-residue/terminal, name/accession converter behaves on the
actual modification dictionary MetaMorpheus loads, de-risking integration.

Phase 10: Layer-2 converter (slice 4, real-Unimod validation).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds ToProFormaString/ToProFormaTerm extension methods on
IBioPolymerWithSetMods so a scored peptide or proteoform converts to its
ProForma 2.0 string in one call. This is the surface MetaMorpheus top-down
output will consume; it composes the existing converter and writer.

End-to-end tests build a PeptideWithSetModifications from its native mzLib
FullSequence and assert the ProForma output for both a name modification
([Oxidation]) and an accession-bearing one ([UNIMOD:35]).

Phase 10: Layer-2 converter (slice 5, peptide bridge).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the "ProForma" column constant to SpectrumMatchFromTsvHeader so search
output can carry the ProForma 2.0 string alongside Full Sequence. The value
is populated by MetaMorpheus's PsmTsvWriter via IBioPolymerWithSetMods
.ToProFormaString(); the column appears on the standard PSM/proteoform path
(crosslink/glyco use their own headers).

Phase 10: ProForma output column.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds SpectrumMatchFromTsv.ProForma, registers the column in ParseHeader, and
reads it via GetOptionalValue so it round-trips with the writer. The column is
optional (IndexOf returns -1 for pre-ProForma files, treated as absent), so
older result files still read unchanged.

In the ambiguity-disambiguation constructor the parent value is carried as-is
rather than split on '|', because ProForma uses '|' as an internal descriptor
separator and cannot be split per candidate.

Phase 10: ProForma reader round-trip.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@trishorts

Copy link
Copy Markdown
Contributor Author

Companion PR (consumes this): smith-chem-wisc/MetaMorpheus#2663

@codecov

codecov Bot commented May 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 38 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.77%. Comparing base (78275fb) to head (b190b65).
⚠️ Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
mzLib/Readers/ProForma/ProFormaConverter.cs 83.05% 11 Missing and 19 partials ⚠️
mzLib/Readers/ProForma/ProFormaMultiTermReader.cs 93.18% 2 Missing and 4 partials ⚠️
mzLib/Readers/ProForma/ProFormaMultiTerm.cs 87.50% 1 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1072      +/-   ##
==========================================
+ Coverage   82.33%   86.77%   +4.44%     
==========================================
  Files         382      396      +14     
  Lines       50509    51137     +628     
  Branches     6102     6236     +134     
==========================================
+ Hits        41585    44373    +2788     
+ Misses       7734     5547    -2187     
- Partials     1190     1217      +27     
Files with missing lines Coverage Δ
...ts/IndividualResultRecords/SpectrumMatchFromTsv.cs 89.05% <100.00%> (+0.09%) ⬆️
...ividualResultRecords/SpectrumMatchFromTsvHeader.cs 100.00% <ø> (ø)
...ernalResults/ResultFiles/SpectrumMatchTsvReader.cs 97.31% <100.00%> (+0.01%) ⬆️
mzLib/Readers/ProForma/ProFormaExtensions.cs 100.00% <100.00%> (ø)
mzLib/Readers/ProForma/ProFormaMultiTermWriter.cs 100.00% <100.00%> (ø)
mzLib/Readers/ProForma/ProFormaReader.cs 100.00% <100.00%> (ø)
mzLib/Readers/ProForma/ProFormaWriter.cs 100.00% <100.00%> (ø)
mzLib/Readers/ProForma/ProFormaMultiTerm.cs 87.50% <87.50%> (ø)
mzLib/Readers/ProForma/ProFormaMultiTermReader.cs 93.18% <93.18%> (ø)
mzLib/Readers/ProForma/ProFormaConverter.cs 83.05% <83.05%> (ø)

... and 18 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@trishorts trishorts marked this pull request as ready for review May 24, 2026 23:16
trishorts and others added 5 commits May 24, 2026 19:44
Add 7 NUnit tests for the fail-loud paths in ProFormaConverter that had no
patch coverage:
- the five EnsureLayer2Supported guards (labile, unlocalized, global,
  position-range, sequence-ambiguity modifications)
- the unresolved-terminal-mod throw (N- and C-terminus)
- the terminal name-resolution fallback (mod stored with a residue motif
  rather than the wildcard X)

Lifts ProFormaConverter.cs patch coverage from ~80% well above 90%. The two
lines left uncovered (SelectCandidate count==1 fallback, IsTerminusCompatible
default arm) are provably unreachable from the public API.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…der-writer

# Conflicts:
#	mzLib/Readers/Readers.csproj
Fixes from the PR smith-chem-wisc#1072 review of the ProForma Layer-2 converter:

- Match the motif's modified residue instead of the whole motif string, so
  context-bearing motifs (e.g. the N-glyc sequon Nxs) resolve instead of
  throwing MzLibException for an otherwise-known modification.
- Cache the accession index per allModsKnown instance (ConditionalWeakTable)
  rather than rebuilding it over the full mod database on every term, removing
  O(mods x references) work per converted PSM in batch consumption.
- Fail loud (MzLibException) when two tags target the same residue index
  instead of silently overwriting one, per the converter's stated contract.
- Throw a descriptive MzLibException, not a raw IndexOutOfRangeException, for
  an out-of-range tag index.
- Emit ontology accessions in a fixed UNIMOD > MOD > RESID order so
  ToProFormaString output is deterministic when a mod has multiple references.
- Guard null arguments in ToModificationDictionary/ToProFormaTerm.

Addresses 6 findings (2 Major, 4 Minor) from the PR smith-chem-wisc#1072 review.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fixes from the PR smith-chem-wisc#1072 review of the ProForma test suite:

- Assert the exact v2-spec valid corpus count (EqualTo(89)) instead of a
  weak GreaterThan(70), matching the precise-count sibling tests so a
  deleted or mistyped row is caught.
- Assert the crosslink guard's message ("tag group") so the test confirms
  the tag-group path fired rather than passing on an incidental missing-mod
  throw against the empty dictionary.
- Use a unique per-test temp filename (Test.ID) instead of a fixed name in
  the shared test directory, removing a parallelization/leftover hazard.

Addresses 3 Minor findings from the PR smith-chem-wisc#1072 review.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
trishorts and others added 4 commits June 19, 2026 10:55
…-chain forms

Add a layer above the single-term TopDownProteomics parser so ProForma 2.0
constructs that live above one proteoform term round-trip losslessly:
chimeric peptidoforms (+), charge states (/z[adducts]), and inter-chain
crosslink / branch chains (//).

- ProFormaMultiTerm / ProFormaPeptidoform: model a chimeric set of peptidoforms,
  each one or more // chains plus an optional /charge[adducts].
- ProFormaMultiTermReader / ProFormaMultiTermWriter: split on the top-level
  +, /, and // operators (bracket-depth-aware), delegating each chain to the
  wrapped single-term ProFormaReader/ProFormaWriter and reassembling on write.

Covers the 13 corpus forms previously pinned as "above a single term"
(charge x7, chimeric x1, inter-chain crosslink x3, branch x2). 17 new
multi-term round-trip tests pass; the single-term parser is unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JNnpzBNwfnTbZxtZdtAzLG
The 0.0.299 ProFormaWriter now serializes a range bearing multiple
modifications (spec 4.5), closing the only known SDK writer gap. The
KnownGap_MultiModRange guard test fired as designed: drop it, remove
v2-4.5-01 from the writer-gap exclusion set, and let that example flow
through the standard round-trip suite. 144 ProForma tests green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Treat the wildcard "X" motif as matching any residue, so an interior
  mod stored against an X-motif Modification resolves on read instead of
  throwing (previously broke the round-trip for that whole class of mods).
- Accept "Anywhere." LocationRestriction at a terminus so a terminal-
  position mod that ToProFormaTerm writes is also resolvable on read.
- Reject IonAdducts without a Charge in the ProFormaPeptidoform ctor,
  since the writer can only emit adducts inside /z[...] and would
  otherwise drop them silently.

Addresses 3 findings (1 Major, 2 Minor) from the PR smith-chem-wisc#1072 review.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Assert exact input preservation for an inter-chain crosslink form and
  a branch // form, plus chain-sequence content for the chimeric and
  multi-chain splits; idempotence alone would miss a consistent
  drop/reorder of chains or crosslink labels.
- ProFormaTestCorpus.Load() throws InvalidDataException on a malformed
  row instead of calling NUnit Assert, so a bad corpus fails cleanly at
  execution rather than as a TestCaseSource discovery error.

Addresses 3 findings from the PR smith-chem-wisc#1072 review (fix_007/008/009 were
already fixed in-branch; fix_010 informational, skipped).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@trishorts

Copy link
Copy Markdown
Contributor Author

Manual unit-test review

All 36 unit test(s) added by this PR were reviewed individually by the author (not only by automated/AI-generated checks): 33 approved as written, 3 edited, 0 flagged.

This satisfies the author's standing AI-use policy that AI-assisted tests receive human review before relying on them. Recorded 2026-06-30 15:14.

Pins the fix that lets IsTerminusCompatible accept an "Anywhere."
LocationRestriction at a terminus: ToProFormaTerm writes any position-1
mod as an N-terminal descriptor regardless of restriction, so the read
path must resolve it. Fails before that change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant