From 20a76ea900705be87dff09400d719894c92fdb98 Mon Sep 17 00:00:00 2001 From: trishorts Date: Fri, 19 Jun 2026 11:07:06 -0500 Subject: [PATCH 1/2] fix ProForma writer to serialize a range bearing multiple modifications The writer threw "Can't nest ranges within each other" for a range carrying more than one modification (e.g. "(SEQ)[mod1][mod2]", ProForma 2.0 section 4.5). Each modification on a range is parsed as a separate tag spanning the same start/end, and the writer's internal-tag loop mistook those co-located modifications for nested ranges. Treat a tag whose start and end match the current range as another modification on that range and emit it as a consecutive descriptor after the range closes; genuine nested ranges still throw. Adds a constructed unit test and a parse/write round-trip regression test; full suite green (235 passed). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01JNnpzBNwfnTbZxtZdtAzLG --- .../ProForma/ProFormaWriter.cs | 35 +++++++++++++++---- .../ProForma/ProFormaWriterTests.cs | 29 +++++++++++++++ 2 files changed, 57 insertions(+), 7 deletions(-) diff --git a/src/TopDownProteomics/ProForma/ProFormaWriter.cs b/src/TopDownProteomics/ProForma/ProFormaWriter.cs index 06f2241..b650262 100644 --- a/src/TopDownProteomics/ProForma/ProFormaWriter.cs +++ b/src/TopDownProteomics/ProForma/ProFormaWriter.cs @@ -103,13 +103,18 @@ void WriteTagOrGroup(object obj, StringBuilder sb, bool displayValue, double wei bool hasAmbiguousSequence = obj is ProFormaTag tag2 && tag2.HasAmbiguousSequence; + // Additional tags that share this exact range are extra modifications on the same + // range (e.g. "(SEQ)[mod1][mod2]"); they are written after the range closes and are + // not nested ranges. + List<(object, int, int, bool, double)>? sameRangeTags = null; + if (startIndex == endIndex && !hasAmbiguousSequence) { // Write sequence up to tag sb.Append(term.Sequence.Substring(currentIndex, startIndex - currentIndex + 1)); currentIndex = startIndex + 1; } - else // Handle ambiguity range + else // Handle a range (ambiguity range, or a range bearing one or more modifications) { // Write sequence up to range (checking for internal tags) sb.Append(term.Sequence[currentIndex..startIndex]); @@ -121,19 +126,29 @@ void WriteTagOrGroup(object obj, StringBuilder sb, bool displayValue, double wei if (hasAmbiguousSequence) sb.Append('?'); - // Check for other tags that might be inside this range + // Check for other tags that fall within this range int j = i + 1; while (j < tagsAndGroups.Count && tagsAndGroups[j].Item2 <= endIndex) { (object, int, int, bool, double) internalTag = tagsAndGroups[j]; - if (internalTag.Item2 != internalTag.Item3) + if (internalTag.Item2 == startIndex && internalTag.Item3 == endIndex) + { + // Another modification on the same range; emit it after the ')'. + (sameRangeTags ??= new List<(object, int, int, bool, double)>()).Add(internalTag); + } + else if (internalTag.Item2 != internalTag.Item3) + { throw new ProFormaParseException("Can't nest ranges within each other."); + } + else + { + // A single-residue tag located inside the range. + sb.Append(term.Sequence[currentIndex..(internalTag.Item2 + 1)]); + currentIndex = internalTag.Item2 + 1; - sb.Append(term.Sequence[currentIndex..(internalTag.Item2 + 1)]); - currentIndex = internalTag.Item2 + 1; - - WriteTagOrGroup(internalTag.Item1, sb, internalTag.Item4, internalTag.Item5); + WriteTagOrGroup(internalTag.Item1, sb, internalTag.Item4, internalTag.Item5); + } j++; i++; @@ -144,6 +159,12 @@ void WriteTagOrGroup(object obj, StringBuilder sb, bool displayValue, double wei } WriteTagOrGroup(obj, sb, displayValue, weight); + + if (sameRangeTags != null) + { + foreach (var extra in sameRangeTags) + WriteTagOrGroup(extra.Item1, sb, extra.Item4, extra.Item5); + } } // Write the rest of the sequence diff --git a/tests/TopDownProteomics.Tests/ProForma/ProFormaWriterTests.cs b/tests/TopDownProteomics.Tests/ProForma/ProFormaWriterTests.cs index 46c3502..203d1f7 100644 --- a/tests/TopDownProteomics.Tests/ProForma/ProFormaWriterTests.cs +++ b/tests/TopDownProteomics.Tests/ProForma/ProFormaWriterTests.cs @@ -365,5 +365,34 @@ public void WriteSequenceAmbiguities() Assert.AreEqual("SE(?Q)UENCE", result); } + + [Test] + public void WriteMultipleModificationsOnSameRange() + { + // A range bearing more than one modification: "(SEQ)[mod1][mod2]" (ProForma 2.0 section 4.5). + // Each modification is a separate tag spanning the same range; they must be emitted as + // consecutive descriptors after the range, not treated as nested ranges. + var term = new ProFormaTerm("SEQUENCE", tags: new[] + { + new ProFormaTag(2, 5, new[] { new ProFormaDescriptor(ProFormaKey.Mass, "+14.05") }), + new ProFormaTag(2, 5, new[] { new ProFormaDescriptor(ProFormaKey.Name, "Oxidation") }) + }); + var result = _writer.WriteString(term); + + Assert.AreEqual("SE(QUEN)[+14.05][Oxidation]CE", result); + } + + [Test] + public void RoundTripMultipleModificationsOnRange() + { + // Regression for the "Can't nest ranges within each other" writer bug on a range that + // carries several modifications (ProForma 2.0 section 4.5). + var parser = new ProFormaParser(); + string proForma = "PRT(ESFRMS)[Oxidation][Oxidation][half cystine][half cystine]ISK"; + + string written = _writer.WriteString(parser.ParseString(proForma)); + + Assert.AreEqual(proForma, written); + } } } \ No newline at end of file From 291403e333716eab14ad48e52d2d617af174a0e9 Mon Sep 17 00:00:00 2001 From: trishorts Date: Tue, 30 Jun 2026 10:19:12 -0500 Subject: [PATCH 2/2] test: cover nested-range guard and point-mod-inside-range in writer Adds two ProFormaWriterTests exercising the branches introduced by the multi-mod-range fix that were previously uncovered (codecov/patch): - WriteSingleResidueModificationInsideRange: a point modification localized to one residue inside a range emits within the parentheses. - WriteNestedRangesThrows: a genuine nested range (distinct start/end) is still rejected with ProFormaParseException. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ProForma/ProFormaWriterTests.cs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/TopDownProteomics.Tests/ProForma/ProFormaWriterTests.cs b/tests/TopDownProteomics.Tests/ProForma/ProFormaWriterTests.cs index 203d1f7..1fc46bf 100644 --- a/tests/TopDownProteomics.Tests/ProForma/ProFormaWriterTests.cs +++ b/tests/TopDownProteomics.Tests/ProForma/ProFormaWriterTests.cs @@ -394,5 +394,35 @@ public void RoundTripMultipleModificationsOnRange() Assert.AreEqual(proForma, written); } + + [Test] + public void WriteSingleResidueModificationInsideRange() + { + // A point modification localized to one residue that sits inside a range: + // the inner residue tag is written within the range parentheses, the range + // descriptor after the closing ')'. + var term = new ProFormaTerm("SEQUENCE", tags: new[] + { + new ProFormaTag(1, 5, new[] { new ProFormaDescriptor(ProFormaKey.Mass, "+14.05") }), + new ProFormaTag(3, new[] { new ProFormaDescriptor(ProFormaKey.Name, "Oxidation") }) + }); + var result = _writer.WriteString(term); + + Assert.AreEqual("S(EQU[Oxidation]EN)[+14.05]CE", result); + } + + [Test] + public void WriteNestedRangesThrows() + { + // A genuine range nested inside another range (distinct start/end pairs) is invalid + // ProForma and must still be rejected. + var term = new ProFormaTerm("SEQUENCE", tags: new[] + { + new ProFormaTag(1, 5, new[] { new ProFormaDescriptor(ProFormaKey.Mass, "+14.05") }), + new ProFormaTag(2, 4, new[] { new ProFormaDescriptor(ProFormaKey.Name, "Oxidation") }) + }); + + Assert.Throws(() => _writer.WriteString(term)); + } } } \ No newline at end of file