Clean up msms characterization by mikikot113 · Pull Request #686 · systemsomicslab/MsdialWorkbench

mikikot113 · 2025-12-23T04:32:06Z

In lipidomics analysis, annotation methods for several lipid classes were modified.

…terization

Copilot

Pull request overview

This PR refactors the lipidomics MS/MS characterization code to improve annotation methods for several lipid classes. The changes primarily focus on correcting boolean logic, renaming parameters for clarity, adjusting fragment matching criteria, and adding support for additional adduct types.

Fixed multiple boolean logic errors in validation conditions across various ceramide characterization methods
Renamed parameters from "Omegaacyl/ExtAcyl" to "Acyl" for consistency and clarity
Modified fragment matching thresholds and queries for several lipid classes (DGDG, Cer_EOS, Cer_EODS, HexCer_EOS, Cer_EBDS)

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 18 comments.

File	Description
tests/Common/CommonStandardTests/Lipidomics/MsmsCharacterizationTests.cs	Added two new test methods for CerEODS and HexCerEOS lipid characterization
src/Common/CommonStandard/Resources/LbmQueries.txt	Added support for DGDG with [M+Na]+ adduct and Cer_NP with positive ion mode adducts
src/Common/CommonStandard/Lipidomics/MsmsCharacterization.cs	Extensive refactoring of characterization logic including boolean condition fixes, parameter renaming, threshold adjustments, and algorithm modifications across multiple lipid classes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-23T04:42:54Z

src/Common/CommonStandard/Lipidomics/MsmsCharacterization.cs

                    var isClassIonFound = LipidMsmsCharacterizationUtility.isDiagnosticFragmentExist(spectrum, ms2Tolerance, diagnosticMz, threshold);
                    var isClassIon2Found = LipidMsmsCharacterizationUtility.isDiagnosticFragmentExist(spectrum, ms2Tolerance, diagnosticMz2, threshold2);
-                    if (isClassIonFound == !true || isClassIon2Found == !true) return null;
+                    if (isClassIonFound == false && isClassIon2Found == false) return null;


The original condition had incorrect syntax "isClassIonFound == !true || isClassIon2Found == !true". The fix corrects it but changes from OR to AND. If the original intent was to require both ions (OR logic with negation), it should be "isClassIonFound == false || isClassIon2Found == false". Verify correct behavior for Ceramide NP.

Copilot · 2025-12-23T04:42:54Z

src/Common/CommonStandard/Lipidomics/MsmsCharacterization.cs

                    var isClassIon1Found = LipidMsmsCharacterizationUtility.isDiagnosticFragmentExist(spectrum, ms2Tolerance, diagnosticMz1, threshold1);
                    var isClassIon2Found = LipidMsmsCharacterizationUtility.isDiagnosticFragmentExist(spectrum, ms2Tolerance, diagnosticMz2, threshold2);
-                    if (isClassIon1Found != true || isClassIon2Found != true) return null;
+                    if (isClassIon1Found == false && isClassIon2Found == false) return null;


The logic change from OR to AND with negation may alter the intended behavior. The original condition returned null if either ion was NOT found. The new condition returns null only if BOTH ions are NOT found. This means the function will now proceed when at least one ion is found, whereas previously it required both ions to be present. This is a significant behavioral change that should be verified as intentional.

Copilot · 2025-12-23T04:42:54Z

src/Common/CommonStandard/Lipidomics/MsmsCharacterization.cs

                    var isClassIonFound = LipidMsmsCharacterizationUtility.isDiagnosticFragmentExist(spectrum, ms2Tolerance, diagnosticMz, threshold);
                    var isClassIon2Found = LipidMsmsCharacterizationUtility.isDiagnosticFragmentExist(spectrum, ms2Tolerance, diagnosticMz2, threshold2);
-                    if (isClassIonFound == !true || isClassIon2Found == !true) return null;
+                    if (isClassIonFound == false && isClassIon2Found == false) return null;


Same issue as line 10598. The original condition had incorrect syntax "isClassIonFound == !true || isClassIon2Found == !true". While the fix corrects the syntax error, it changes the logic from OR to AND. If the original intent was to require both ions (return null when either is missing), it should be "isClassIonFound == false || isClassIon2Found == false". Verify the correct behavior for Ceramide ABP.

Copilot · 2025-12-23T04:42:55Z

src/Common/CommonStandard/Lipidomics/MsmsCharacterization.cs

                    var isClassIon1Found = LipidMsmsCharacterizationUtility.isDiagnosticFragmentExist(spectrum, ms2Tolerance, diagnosticMz1, threshold1);
                    var isClassIon2Found = LipidMsmsCharacterizationUtility.isDiagnosticFragmentExist(spectrum, ms2Tolerance, diagnosticMz2, threshold2);
-                    if (isClassIon1Found != true && isClassIon2Found != true) return null;
+                    if (isClassIon1Found == false || isClassIon2Found == false) return null;


The logic has been inverted. The original "isClassIon1Found != true && isClassIon2Found != true" returned null only if BOTH ions were missing. The new "isClassIon1Found == false || isClassIon2Found == false" returns null if EITHER ion is missing. This changes from requiring at least one ion to requiring both ions. Verify this is the intended behavior for Ceramide AS.

src/Common/CommonStandard/Lipidomics/MsmsCharacterization.cs

Copilot · 2025-12-23T04:42:57Z

tests/Common/CommonStandardTests/Lipidomics/MsmsCharacterizationTests.cs

+            var PRECURSORTYPE = "[M-H]-";
+            var target = new MSScanProperty
+            {
+                PrecursorMz = 736.6824,


The PrecursorMz value in the test target (736.6824) does not match the expected precursor mass for HexCer 34:1;O3(FA 12:0). The molMsRef correctly uses 896.7196, which should also be used in the target MSScanProperty to ensure the test is validating the correct scenario.

Suggested change

PrecursorMz = 736.6824,

PrecursorMz = 896.7196,

src/Common/CommonStandard/Resources/LbmQueries.txt

Copilot · 2025-12-23T04:42:58Z

src/Common/CommonStandard/Lipidomics/MsmsCharacterization.cs

                    var isClassIonFound = LipidMsmsCharacterizationUtility.isDiagnosticFragmentExist(spectrum, ms2Tolerance, diagnosticMz, threshold);
                    var isClassIon2Found = LipidMsmsCharacterizationUtility.isDiagnosticFragmentExist(spectrum, ms2Tolerance, diagnosticMz2, threshold2);
-                    if (isClassIonFound == !true || isClassIon2Found == !true) return null;
+                    if (isClassIonFound == false && isClassIon2Found == false) return null;


Same issue as lines 10598 and 10767. The original condition had incorrect syntax "isClassIonFound == !true || isClassIon2Found == !true". The fix corrects the syntax but changes the logic from OR to AND. Verify the correct behavior for Hexceramide AP - should it require both ions or at least one ion?

Copilot · 2025-12-23T04:42:58Z

src/Common/CommonStandard/Lipidomics/MsmsCharacterization.cs

                    var isClassIon1Found = LipidMsmsCharacterizationUtility.isDiagnosticFragmentExist(spectrum, ms2Tolerance, diagnosticMz1, threshold1);
                    var isClassIon2Found = LipidMsmsCharacterizationUtility.isDiagnosticFragmentExist(spectrum, ms2Tolerance, diagnosticMz2, threshold2);
-                    if (isClassIon1Found != true && isClassIon2Found != true) return null;
+                    if (isClassIon1Found == false || isClassIon2Found == false) return null;


Same logic inversion as line 11585. The original required at least one ion to be present. The new condition requires both ions to be present. Verify this is intentional for Ceramide ADS.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…nto Clean_up_MsmsCharacterization

mikiko.takahashi added 10 commits December 16, 2025 10:04

add DGDG[M+Na]+

cddc7da

update DGDG annotation

cc234df

update

197826b

fix Cer_EBDS, Cer_EODS, Cer_EOS, HexCer_EOS

0247499

fix Cer_EBDS annotation method

1c06836

fix HexCer_EOS annotation method

a555f78

minor

e120203

minor

dd6b3c3

Add MG [M+H]+

899e1fd

Merge remote-tracking branch 'origin/master' into Clean_up_MsmsCharac…

2a30719

…terization

YukiMatsuzawa requested a review from Copilot December 23, 2025 04:32

Copilot started reviewing on behalf of YukiMatsuzawa December 23, 2025 04:33 View session

Copilot AI reviewed Dec 23, 2025

View reviewed changes

mikikot113 and others added 3 commits December 23, 2025 14:24

Update src/Common/CommonStandard/Resources/LbmQueries.txt

352404a

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Fix

be19472

Merge remote-tracking branch 'origin/Clean_up_MsmsCharacterization' i…

bc21497

…nto Clean_up_MsmsCharacterization

YukiMatsuzawa merged commit 17d56be into master Dec 23, 2025
9 checks passed

YukiMatsuzawa deleted the Clean_up_MsmsCharacterization branch December 23, 2025 06:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Clean up msms characterization#686

Clean up msms characterization#686
YukiMatsuzawa merged 13 commits intomasterfrom
Clean_up_MsmsCharacterization

mikikot113 commented Dec 23, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

Conversation

mikikot113 commented Dec 23, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants