Repository navigation
Conversation
SetAllFileSpecificCommonParams rebuilds DigestionParams from an explicit list of fields whenever a spectra file has its own settings, and its own comment warns that a field left out of that list "silently falls back to its constructor default for every file with file-specific settings". mzLib's RespectCleavageBlockingModifications was left out, and its default is false, so any file with any file-specific setting -- even one that only changes a mass tolerance -- got the historical modification-blind digest while the task reported the corrected one. The flag also feeds the index cache fingerprint, so the two runs are not distinguishable afterwards. FileSpecificDigestionParamsTests already claimed to guard against exactly this by comparing the whole object with DigestionParams.Equals, and it did not fire: the rebuild's fallback IS the constructor default, so a field the fixture leaves at its default agrees by accident. The fixture now sets the flag true, which makes that guard real, and the remarks say so for the next field. A named test covers both states so the failure report says which setting was dropped. Requires the mzLib change that adds the flag (mzLib smith-chem-wisc#1119); draft until that merges and ships. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Clean merge; no conflicts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-cleavage-blocking-flag # Conflicts: # MetaMorpheus/Test/FileSpecificDigestionParamsTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SetAllFileSpecificCommonParamsrebuilds a task'sDigestionParamsfrom an explicit list of fields whenever a spectra file has its own settings. Its own comment says what happens to a field left off that list:mzLib's
DigestionParams.RespectCleavageBlockingModificationsis left off it. Its default isfalse, so any file with any file-specific setting — even one that only changes a precursor tolerance — gets the historical modification-blind digest while the task reports the corrected one. The flag also reachesIndexingEngine.ToString(), so the two runs are not distinguishable from the output afterwards.One argument closes it:
The guard that was supposed to catch this, and why it didn't
FileSpecificDigestionParamsTestswas written for exactly this class of bug — the glycopeptide and SILAC flags were dropped the same way — and its second test compares the whole object withDigestionParams.Equals, whose remarks promise that "a digestion setting added in the future that is forgotten in the rebuild fails here too."It did not fail. A whole-object comparison only guards a field the fixture sets away from its constructor default, because the rebuild's fallback is the constructor default, so a field left at its default agrees by accident. The fixture now sets the flag true, which makes that guard real, and the remarks say so in as many words for whoever adds the next field.
Tests
FileSpecificRebuild_WithoutDigestionOverrides_KeepsEveryDigestionSetting— fixture extended, so the existing whole-object guard actually fires.FileSpecificRebuild_KeepsRespectCleavageBlockingModifications(true|false)— new, so the failure report names the setting that was dropped rather than printing two long strings.Mutation-checked. With the one argument removed and everything else unchanged, both the
truecase and the whole-object test fail — the latter with4,Cleave,6,40,256,3,trypsin,Semi,C,False,True,True,True became 4,Cleave,6,40,256,3,trypsin,Semi,C,False,True,True,False. Thefalsecase passes throughout, so the set cannot pass or fail vacuously.How this was run, and what it is waiting on
Draft. It needs the mzLib change that adds the flag, which is mzLib smith-chem-wisc#1119; MetaMorpheus's
mzLibPackageReference has to move past that release before this compiles on CI.Locally it was built against mzLib packed from smith-chem-wisc#1119 at its current head, plus mzLib smith-chem-wisc#1330 — without smith-chem-wisc#1330 MetaMorpheus master does not compile at all against any mzLib that carries smith-chem-wisc#1318's abstract
IDigestionParams.SpecificDigestionAgent. With that package: MetaMorpheus builds with 0 errors, the targeted sweep overFileSpecificDigestionParams|Toml|DigestionAgentName|ParameterTestis 61 passed / 0 failed, and the full MetaMorpheus suite is 2313 passed / 0 failed / 1 skipped.That full-suite number is worth one more sentence, because it is the first time MetaMorpheus's tests have run against this mzLib at all: master's
integrationjob has been unable to compile MetaMorpheus since mzLib smith-chem-wisc#1318 merged, so nothing has exercised the pair. It also required mzLib smith-chem-wisc#1318's toml regression to be fixed, which MetaMorpheus smith-chem-wisc#2835 does — this build carries that PR's twoSpecificDigestionAgentIgnorePropertycalls but deliberately not its.IgnoreProperty(p => p.SpecificProtease), and all 2314 tests pass without it. That is the measurement behind the finding I left on smith-chem-wisc#2835.Un-draft and re-run once smith-chem-wisc#1119 merges and ships.
🤖 Generated with Claude Code