Fix bug where Skyline sometimes ignores the largest detected peak

pwiz_tools/Skyline/Model/Results/ChromDataSet.cs

@@ -618,15 +618,15 @@ public void PickChromatogramPeaks(double[] retentionTimes, bool isAlignedTimes,
             }
             else if (retentionTimes.Length == 0)
             {
-                // Be sure not to remove anything with a higher combined score than
+                // Be sure not to remove anything with as high a combined score as
                 // what happen to look visually like the biggest peaks.
                 double minKeepScore = _listPeakSets.Take(iRemove).Min(peakSet => peakSet.CombinedScore);
 
                 // Backward compatibility: before peptide IDs were integrated
                 // this sorting happened before peaks were extended.
                 _listPeakSets.Sort(ComparePeakLists);
 
-                iRemove = Math.Max(iRemove, _listPeakSets.IndexOf(peakSet => peakSet.CombinedScore == minKeepScore));
+                iRemove = Math.Max(iRemove, _listPeakSets.IndexOf(peakSet => peakSet.CombinedScore == minKeepScore) + 1);
             }
             else
             {

pwiz_tools/Skyline/TestFunctional/PickChromatogramPeaksTest.cs

@@ -0,0 +1,70 @@
+/*
+ * Original author: Nicholas Shulman <nicksh .at. u.washington.edu>,
+ *                  MacCoss Lab, Department of Genome Sciences, UW
+ *
+ * Copyright 2025 University of Washington - Seattle, WA
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+using System;
+using System.Linq;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using pwiz.Common.DataBinding;
+using pwiz.Skyline.Controls.Databinding;
+using pwiz.Skyline.EditUI;
+using pwiz.Skyline.Model.Databinding.Entities;
+using pwiz.SkylineTestUtil;
+
+namespace pwiz.SkylineTestFunctional
+{
+    [TestClass]
+    public class PickChromatogramPeaksTest : AbstractFunctionalTest
+    {
+        [TestMethod]
+        public void TestPickChromatogramPeaks()
+        {
+            TestFilesZip = @"TestFunctional\PickChromatogramPeaksTest.zip";
+            RunFunctionalTest();
+        }
+
+        protected override void DoTest()
+        {
+            RunUI(()=>SkylineWindow.OpenFile(TestFilesDir.GetTestPath("PickChromatogramPeaksTest.sky")));
+            ImportResultsFile(TestFilesDir.GetTestPath("PickPeakTest.wiff"));
+            RunDlg<FindNodeDlg>(SkylineWindow.ShowFindNodeDlg, findNodeDlg =>
+            {
+                findNodeDlg.SearchString = "GQLPSGSSQFPHGQK";
+                findNodeDlg.FindNext();
+                findNodeDlg.Close();
+            });
+            RunUI(()=>SkylineWindow.ShowCandidatePeaks());
+            var candidatePeaks = FindOpenForm<CandidatePeakForm>();
+            WaitForConditionUI(() => candidatePeaks.IsComplete);
+            RunUI(() =>
+            {
+                var databoundGridControl = candidatePeaks.DataboundGridControl;
+                Assert.AreEqual(4, databoundGridControl.RowCount);
+                var colPeakGroupRetentionTime =
+                    databoundGridControl.FindColumn(
+                        PropertyPath.Root.Property(nameof(CandidatePeakGroup.PeakGroupRetentionTime)));
+                Assert.IsNotNull(colPeakGroupRetentionTime);
+                var retentionTimes = Enumerable.Range(0, databoundGridControl.RowCount).Select(iRow =>
+                    Math.Round((double) databoundGridControl.DataGridView.Rows[iRow].Cells[colPeakGroupRetentionTime.Index].Value, 2)).ToList();
+                Assert.AreEqual(7.2, retentionTimes[0]);
+                Assert.AreEqual(7.73, retentionTimes[1]);
+                Assert.AreEqual(8.44, retentionTimes[2]);
+                Assert.AreEqual(8.98, retentionTimes[3]);
+            });
+        }
+    }
+}

pwiz_tools/Skyline/TestFunctional/TestFunctional.csproj

@@ -217,6 +217,7 @@
     <Compile Include="PeakAreaRelativeAbundanceGraphTest.cs" />
     <Compile Include="PeakAreaPeptideGraphTest.cs" />
     <Compile Include="PeakMetricsTest.cs" />
+    <Compile Include="PickChromatogramPeaksTest.cs" />
     <Compile Include="PrmCeOptimizationTest.cs" />
     <Compile Include="QcTraceChromatogramTest.cs" />
     <Compile Include="QcTraceNameTest.cs" />

pwiz_tools/Skyline/TestTutorial/ExistingExperimentsTutorialTest.cs

@@ -732,7 +732,7 @@ private static void TestApplyToAll()
             RunUI(() =>
             {
                 PeakMatcherTestUtil.SelectAndApplyPeak("ESDTSYVSLK", 564.7746, "A_02", false, false, 18.34195);
-                PeakMatcherTestUtil.VerifyPeaks(MakeVerificationDictionary(18.34, 18.34, 18.28, 18.28));
+                PeakMatcherTestUtil.VerifyPeaks(MakeVerificationDictionary(18.3419, 18.34195, 18.27585, 23.481));
             });
             RunUI(() =>
             {

pwiz_tools/Skyline/TestTutorial/GroupedStudies1TutorialTest.cs

@@ -368,7 +368,7 @@ private void ExploreTopPeptides()
                 OkDialog(findDlg, findDlg.Close);
 
                 var findView = WaitForOpenForm<FindResultsForm>();
-                int expectedItems = IsFullData ? 228 : 151;
+                int expectedItems = IsFullData ? 230 : 153;
                 try
                 {
                     WaitForConditionUI(1000, () => findView.ItemCount == expectedItems);
@@ -610,9 +610,9 @@ private void AddTruncatedPrecursorsView(DocumentGridForm documentGrid, bool init
             OkDialog(viewEditor, viewEditor.OkDialog);
 
             var pathTruncated = PropertyPath.Parse("Results!*.Value.CountTruncated");
-            int expectedItems = 86;
+            int expectedItems = 88;
             if (IsFullData)
-                expectedItems = 129;
+                expectedItems = 131;
             try
             {
                 WaitForConditionUI(1000, () => documentGrid.RowCount == expectedItems &&
@@ -1155,7 +1155,7 @@ private void PrepareForStatistics()
 
                 PauseForScreenShot<DocumentGridForm>("Document Grid with MissingData field");
 
-                int expectedRows = IsFullData ? 133 : 89;
+                int expectedRows = IsFullData ? 135 : 91;
                 const int expectedRowsAbbreviated = 221; // When not all of the tests are run
                 RunUI(() =>
                 {
@@ -1650,10 +1650,10 @@ private static void TestApplyToAll()
             {
                 PeakMatcherTestUtil.SelectAndApplyPeak("LNDGSQITFEK", null, "D_138_REP1", false, false, 23.5299);
                 PeakMatcherTestUtil.VerifyPeaks(MakeVerificationDictionary(
-                    23.45410, 22.77782, 23.11210, 23.19398, 22.88790, 23.00840,
-                    23.52992, 23.57400, 23.19233, 23.45998, 22.81207, 22.81960,
-                    23.87478, 23.68238, 23.03755, 22.89255, 22.69688, 23.04172,
-                    22.85375, 23.04702, 22.85068, 22.88932, 22.70258, 23.19258));
+                    23.4541, 22.77782, 23.1121, 23.19398, 22.8879, 23.0084, 
+                    23.52992, 23.574, 23.19233, 23.45998, 22.81207, 22.8196, 
+                    23.87478, 23.68238, 23.03755, 22.89255, 22.69688, 23.04172, 
+                    22.85375, 25.51052, 22.85068, 22.88932, 22.70258, 23.19258));
             });
             // Apply to subsequent
             RunUI(() =>
@@ -1699,10 +1699,10 @@ private static void TestApplyToAll()
             {
                 PeakMatcherTestUtil.SelectAndApplyPeak("LGGEEVSVAC[+57.0]K", null, "H_148_REP1", false, false, 13.6641);
                 PeakMatcherTestUtil.VerifyPeaks(MakeVerificationDictionary(
-                    14.30043, 13.79685, 13.79692, 13.79708, 14.33403, 14.90242,
-                    13.83123, 14.03223, 13.66342, 13.76475, 13.83022, 13.73013,
-                    14.33438, 13.83052, 14.70115, 13.66408, 13.63018, 13.69645,
-                    13.73080, 13.52982, 13.69677, 13.83090, 13.56257, 13.76500));
+                    14.30043, 13.79685, 13.79692, 13.79708, 14.33403, 14.90242, 
+                    13.76423, 14.03223, 13.66342, 13.76475, 13.83022, 13.73013, 
+                    14.33438, 13.83052, 14.70115, 13.66408, 13.63018, 13.69645, 
+                    13.7308, 13.52982, 13.69677, 13.8309, 13.56257, 13.765));
             });
 
             // For each test, a peak was picked and applied - undo two actions per test

pwiz_tools/Skyline/TestTutorial/PeakPickingTutorialTest.cs

@@ -99,8 +99,8 @@ private string GetTestPath(string relativePath)
 
         private readonly string[] EXPECTED_COEFFICIENTS =
         {
-            "-0.1095|-0.7689|1.9147|0.9647|0.0265|0.1822|0.2229| null |0.5529|6.5433|-0.0357|0.5285|0.6585| null | null | null | null | null ",
-            "0.2900| null | null |5.9841|-0.0624|0.6681|0.7968| null | null | null | null | null | null | null | null | null | null | null ",
+            "-0.1178|-0.8087|1.9805|0.9576|0.0263|0.1984|0.2404| null |0.3943|6.6709|-0.0364|0.5226|0.6494| null | null | null | null | null ",
+            "0.2725| null | null |6.0285|-0.0646|0.6737|0.8121| null | null | null | null | null | null | null | null | null | null | null ",
         };
 
         protected override void DoTest()
@@ -200,7 +200,7 @@ protected override void DoTest()
             var editDlg = ShowDialog<EditPeakScoringModelDlg>(reintegrateDlg.AddPeakScoringModel);
             RunUI(() => editDlg.TrainModel());
             PauseForScreenShot<EditPeakScoringModelDlg.ModelTab>("Edit Peak Scoring Model form trained model");
-            RunUI(() => Assert.AreEqual(0.5893, editDlg.PeakCalculatorsGrid.Items[3].PercentContribution ?? 0, 0.005));
+            RunUI(() => Assert.AreEqual(0.6065, editDlg.PeakCalculatorsGrid.Items[3].PercentContribution ?? 0, 0.005));
 
             Control selectedGraphControl = null;
             RunUI(() =>

pwiz_tools/Skyline/TestTutorial/TutorialAuditLogs/en/TestGroupedStudies1Tutorial.log

@@ -162,8 +162,8 @@ Undo Redo : Changed peak bounds of NP_036870 > K.DFATVYVDAVK.D [36, 46] > 614.31
 Summary   : Changed peak bounds of NP_036870 > K.DFATVYVDAVK.D [36, 46] > 614.3164++
 All Info  :
 Changed peak bounds of NP_036870 > K.DFATVYVDAVK.D [36, 46] > 614.3164++
-Changed start time of all peaks of NP_036870 > K.DFATVYVDAVK.D [36, 46] > 614.3164++ in "D_108_REP2" from 27.79 to 26.86
-Changed end time of all peaks of NP_036870 > K.DFATVYVDAVK.D [36, 46] > 614.3164++ in "D_108_REP2" from 28.72 to 27.40
+Changed start time of all peaks of NP_036870 > K.DFATVYVDAVK.D [36, 46] > 614.3164++ in "D_108_REP2" from 27.13 to 26.86
+Changed end time of all peaks of NP_036870 > K.DFATVYVDAVK.D [36, 46] > 614.3164++ in "D_108_REP2" from 27.86 to 27.40
 
 Undo Redo : Deleted target NP_036870 > K.FGLYSDQMR.E [182, 190]
 Summary   : Deleted target NP_036870 > K.FGLYSDQMR.E [182, 190]
@@ -338,17 +338,18 @@ Summary   : Set MissingData of DFATVYVDAVK to True
 All Info  :
 Set MissingData of DFATVYVDAVK to True
 
-Undo Redo : Pasted 89 values into the document grid
-Summary   : Pasted 89 values into the document grid
+Undo Redo : Pasted 91 values into the document grid
+Summary   : Pasted 91 values into the document grid
 All Info  :
-Pasted 89 values into the document grid
+Pasted 91 values into the document grid
 Document grid > Report name is "Truncated Precursors"
 Document grid > Column settings > Column Sorts : contains { Column = "Count Truncated", Direction = "Descending" }
 Set MissingData of DFATVYVDAVK to True
 Set MissingData of DFATVYVDAVK to True
 Set MissingData of DFATVYVDAVK to True
 Set MissingData of DFATVYVDAVK to True
 Set MissingData of DFATVYVDAVK to True
+Set MissingData of DFATVYVDAVK to True
 Set MissingData of DYVSQFESSTLGK to True
 Set MissingData of DYVSQFESSTLGK to True
 Set MissingData of DYVSQFESSTLGK to True
@@ -358,6 +359,7 @@ Set MissingData of C[+57.021464]SLPRPWALTFSYGR to True
 Set MissingData of TGTNLMDFLSR to True
 Set MissingData of TGTNLMDFLSR to True
 Set MissingData of TGTNLMDFLSR to True
+Set MissingData of TGTNLMDFLSR to True
 Set MissingData of ASGIIDTLFQDR to True
 Set MissingData of ASGIIDTLFQDR to True
 Set MissingData of SDFQVPC[+57.021464]QYSQQLK to True
@@ -522,6 +524,8 @@ TRUE
 TRUE
 TRUE
 TRUE
+TRUE
+TRUE
 
 Report name = "Truncated Precursors",
 Column settings =