Possbility bug

[trishorts]

PR Changes Overview — ChildScanTolerance Fixes

Date: 2026-04-08
Branch: ChildScanTolerance
Based on: fixes_report.md review (fixes 001-031)
Scope: Critical and select maintenance fixes only

---

Summary

These changes fix several critical bugs in the low-resolution (ion trap) product mass tolerance feature introduced in PR smith-chem-wisc#2635. The core issues were:

1. Wrong default fallback — ProductMassTolerance_LowRes defaulted to a hardcoded AbsoluteTolerance(0.35) instead of inheriting from ProductMassTolerance, silently applying Da tolerance where ppm was expected.
2. No auto-detection of low-res scans — callers had to manually pass isLowRes: true, and most didn't, causing ion trap scans to use the wrong (narrow) tolerance.
3. Incorrect probability calculation — the p value for glycopeptide scoring used the wrong tolerance window for ion trap scans, inflating statistical confidence.
4. GUI unit mismatch — the UI combo box showed "ppm" when the tolerance was null, but saving produced a Da value.
5. Dead code / unnecessary overload — a 22-parameter forwarding overload added maintenance burden with no benefit.

---

Files Changed

1. MetaMorpheus/EngineLayer/CommonParameters.cs

Fix 001 (Critical) — Wrong default fallback for ProductMassTolerance_LowRes

Line	Change
89	productMassTolerance_LowRes ?? new AbsoluteTolerance(0.35) changed to productMassTolerance_LowRes ?? ProductMassTolerance

When no explicit low-res tolerance is provided, it now inherits the resolved ProductMassTolerance value (default 20 ppm) instead of hardcoding 0.35 Da. This ensures backward compatibility — existing configs without a low-res setting behave identically to before the PR.

---

2. MetaMorpheus/EngineLayer/MetaMorpheusEngine.cs

Fix 002 (Critical) — Auto-detect low-res scans from analyzer metadata

Change	Description
MatchFragmentIons signature	Removed bool isLowRes = false parameter
MatchFragmentIons body	Added bool isLowRes = IsLowResolutionScan(scan) auto-detection
MatchFragmentIonsOfAllCharges signature	Removed bool isLowRes = false parameter
MatchFragmentIonsOfAllCharges body	Added bool isLowRes = IsLowResolutionScan(scan) auto-detection
New method IsLowResolutionScan	internal static helper that checks scan.TheScan.MzAnalyzer for IonTrap2D / IonTrap3D

This centralizes the low-res determination so every call site automatically gets the correct tolerance without needing to independently determine the scan type. The helper is internal static so it can be reused by GlycoSearchEngine and tests.

---

3. MetaMorpheus/EngineLayer/GlycoSearch/GlycoSearchEngine.cs

Fix 002 (Critical) — Remove manual isLowRes passing at child scan call site

Line	Change
~335	Removed bool isIonTrapData = ... and isLowRes: isIonTrapData from MatchFragmentIons call (now auto-detected)

Fix 008 (Critical) — Parent scan p calculation uses wrong tolerance

Line	Change
~326	Added var parentProductTolerance = IsLowResolutionScan(theScan) ? ...LowRes : ... before p calculation

Bug fix (build error from fix 002) — Stale isIonTrapData reference in child scan p calculation

Line	Change
~358	Changed isIonTrapData to IsLowResolutionScan(childScan)

---

4. MetaMorpheus/GUI/TaskWindows/GlycoSearchTaskWindow.xaml.cs

Fix 013 (Critical) — UI combo box defaults to ppm when tolerance is null

Line	Change
190	Added `task.CommonParameters.ProductMassTolerance_LowRes == null

When null, the combo box now defaults to "Da" (index 0), consistent with the save fallback behavior.

Fix 014 (Critical) — Save button hardcodes 0.35 Da fallback

Line	Change
374	new AbsoluteTolerance(0.35) changed to ProductMassTolerance

The save fallback now inherits the user's configured product tolerance instead of hardcoding a Da value.

Fix 019 (Maintenance) — Updated CheckTaskSettingsValidity call site

Lines	Change
264-268	Removed productMassTolerance_LowResTextBox.Text from positional arg smith-chem-wisc#3; added it as named arg productMassTolerance_LowRes: at end of call

Required by the signature change in TaskValidator.cs.

---

5. MetaMorpheus/GUI/Util/TaskValidator.cs

Fix 019 (Maintenance) — Remove dead overload, clean up signature

Change	Description
Removed using Org.BouncyCastle.Tls	Unused import
Deleted 22-parameter forwarding overload	Entire method (~50 lines) that just forwarded to the 23-param version with null
Moved productMassTolerance_LowRes to end of signature	Now an optional parameter with = null default

Existing 22-arg callers (XLSearch, Search, Calibrate, GPTMD task windows) continue to compile unchanged. The single caller that passes the low-res value (GlycoSearchTaskWindow) now uses a named argument.

---

6. MetaMorpheus/Test/GlycoSearchEngineTest.cs

Test updates — Aligned assertions with new default behavior

Test	Change
TestLowResToleranceConstruction_Default (line 83)	Now expects PpmTolerance(20) instead of AbsoluteTolerance(0.35)
TestLowResToleranceConstruction_Default2 (line 103)	Now expects PpmTolerance(20) — TOML deserialization constructs with defaults before setting properties
TestLowResToleranceConstruction_Default2 (line 125)	Written TOML round-trip: expects PpmTolerance(20)
LowRes_CalibrateAndSearch (line 330)	Post-calibration: low-res retains original 20 ppm (calibration doesn't update it)
Removed isLowRes: true / isLowRes: false args	Two call sites each — parameter no longer exists

---

Fixes Applied vs. Skipped

Fix	Severity	Status	Reason
001	Critical	Applied	Wrong default fallback
002	Critical	Applied	Missing auto-detection
008	Critical	Applied	Parent scan p uses wrong tolerance
009	Critical	Skipped	Already covered by fix 008
011	Critical	Skipped	User decision — large new feature addition to FindNGlycan
013	Critical	Applied	UI combo box null handling
014	Critical	Applied	Save button hardcoded fallback
017	Critical	Skipped	Refactoring only, no active bug
019	Critical	Applied	Removed dead overload
005-031 (Major/Minor)	Various	Not yet reviewed	Deferred to follow-up

---

Build & Test Status

• All compilation errors resolved
• All previously failing tests updated and passing