Skip to content

Opt-in: treat a cleavage-blocking modification as abolishing its cleavage site (Modification.BlocksCleavage) - #1119

Open
trishorts wants to merge 6 commits into
smith-chem-wisc:masterfrom
trishorts:fix/digest-cleavage-blocking-mods
Open

Opt-in: treat a cleavage-blocking modification as abolishing its cleavage site (Modification.BlocksCleavage)#1119
trishorts wants to merge 6 commits into
smith-chem-wisc:masterfrom
trishorts:fix/digest-cleavage-blocking-mods

Conversation

@trishorts

@trishorts trishorts commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Trypsin cannot cleave after an acylated lysine. Digestion applies modifications after cleavage, so nothing checked whether a modification abolished the very site a peptide was cut at — and PEPTIDEK[acetyl] was reported as a clean, zero-missed-cleavage tryptic peptide describing an event that cannot occur, while the real peptide (PEPTIDEK[acetyl]AAAAAAAR, reading through the blocked site) was scored as a missed cleavage and often not generated at all.

This PR adds Modification.BlocksCleavage and an opt-in DigestionParams.RespectCleavageBlockingModifications. Default is false, which reproduces historical digestion exactly.

What it does

With the flag on, in a full-specificity search, digestion performs a trade:

Protein AAAAAARPEPTKPEPTRAAAAAAK, acetyl-K variable, maxMissedCleavages: 0

flag OFF                          flag ON
  PEPTK                             PEPTK
  PEPTK[Acetyl]      <- impossible  PEPTK[Acetyl]PEPTR   <- the real peptide, MC=0
  PEPTR                             PEPTR

The impossible peptidoform leaves and the read-through that replaces it arrives, so the peptide stays identifiable. Both halves matter, and they are deliberately coupled — see Scope below.

Three pieces make that work:

  1. Modification.BlocksCleavage — classifies a modification as abolishing its cleavage site. Only side-chain (Anywhere.) acylations on K, and citrullination on R, qualify. Charge is the criterion: acylation removes the ε-amine's positive charge, so trypsin cannot cleave; methylation retains it and is therefore excluded (it is a genuine missed cleavage, not an impossible peptide). N-terminal α-amine acetylation on K is correctly not blocking — the side chain stays charged.
  2. The C-terminal drop — a peptidoform whose C-terminal residue carries a blocking modification, where that C-terminus is an internal protease cut, is dropped. A peptide ending at the protein's own C-terminus is exempt: no cleavage happened there for a modification to block.
  3. Missed cleavages count cleavage events, not residues — a blocked residue is not a cleavage site for the peptidoform carrying it, so it is not a missed cleavage either. Digestion enumerates a wider span internally to reach read-through forms, but that slack is discounted again before a peptide is emitted, so MaxMissedCleavages keeps its meaning: no peptide leaves digestion reporting more missed cleavages than the caller asked for.

Scope: what is and is not in this PR

In scope — full-specificity searches only (SearchModeType == CleavageSpecificity.Full).

Explicitly out of scope — semi-specific and nonspecific searches are left entirely untouched, and that is a deliberate design decision rather than an oversight.

The drop and the wider generation span are two halves of one exchange. The slack is granted only in a full search, so applying the drop outside one performs half the trade. Measured, flag off vs on:

mode maxMC dropped added net
Full 0 PEPTK[Acetyl] PEPTK[Acetyl]PEPTR 6 → 6
Semi 0 PEPTK[Acetyl] (none) 6 → 5

In the semi case the peptide becomes unidentifiable — the wrong answer is removed and the right one never generated. That is strictly worse than leaving it alone, so the drop now requires SearchModeType == Full too. It would have bitten whenever the read-through needs more budget than MaxMissedCleavages allows: one blocked site at 0, three consecutive at 2.

Making semi searches benefit properly is follow-up work, and it is not simply widening that gate. A semi peptide's C-terminus may be a genuine protease cut or a length-driven truncation, and telling them apart requires the protease's cleavage-site list — which full digestion gets free from its own enumeration and semi digestion does not.

Known limitations, carried knowingly

  • The classifier is name-keyed. BlocksCleavage matches acyl stems in the modification's name. Broader Unimod coverage and the bluntness of the methyl exclusion are curation refinements. A user-declarable blocking flag persisted in the custom-modification .txt (the BM/BL precedent) is the real upgrade path.
  • Blocked internal sites are counted without re-deriving the protease's site list. A modified K or R that was never a cleavage site under the configured protease can discount a missed cleavage it did not occupy. Harmless for trypsin (K|,R|, which cleaves unconditionally); untested for context-restricted proteases such as trypsin|P. The count is clamped so it cannot go negative, and the C-terminal drop is exact.
  • Fixed blocking modifications remain an unbounded edge: the generation slack is tied to MaxMods, which bounds variable modifications only.
  • Truncation-product ends beyond the protein C-terminus are not exempted from the drop.
  • BlocksCleavage recomputes on each access (lowercases and scans the name) rather than caching.
  • Not exposed to MetaMorpheus. The flag is a DigestionParams constructor parameter only; it is not on IDigestionParams, has no RNA equivalent, and MetaMorpheus has zero references to it, so every MM path takes the false default. Surfacing it needs GUI plumbing and a check that the flag survives the DigestionParameters.toml round-trip through Nett given its private setter.

Review history

Round 2 (02bce317) fixed five real correctness bugs found in a second review pass: N-terminal α-amine acetylation on K misclassified as blocking; Clone() dropping the flag on its non-None path; Digestion() bypassing the mutable missed-cleavage property; a fixed slack of 2 losing peptides when more than two sites were blocked (now MaxMods); and the C-terminal drop firing on non-cut termini.

Round 3 — this round, from @nbollis's two review comments, seconded by @Alexander-Sol:

  • "the number of missed cleavages we get out of digestion may or may not match our digestion parameters"confirmed and fixed. At maxMissedCleavages: 0 the emitted peptides reported < 0, 0, 1 >. The filter was already computing the blocked-discounted count to decide survival, but the peptide was still constructed with the modification-blind count, so the generation slack leaked out. The discounted count is now what the surviving peptidoform carries — which is the alternative both reviewers proposed.
  • "Cleavage specificity == Full will prevent this from working in nonspecific and semi-specific searches, is that intended?"tested, and it was worse than intended. It was not inert in semi searches, it was half-applied; see the table above. Now genuinely confined to full searches, with the reasoning documented at the gate.

Tests

New this round, each verified red against the previous source:

  • ReadThroughOfABlockedSite_ReportsZeroMissedCleavages_NotOne — was 1, now 0.
  • NoEmittedPeptide_EverReportsMoreMissedCleavagesThanRequested(0/1/2) — was < 0, 0, 1 > at a requested max of 0.
  • SemiSpecificSearch_IsEntirelyUnaffectedByTheFlag(0/1/2) — fails on all three budgets without the new gate. This replaces SemiSpecific_NonFullPeptides_AreUnaffectedByTheFlag, which compared only the non-Full subset of a semi digest and therefore stayed green while the Full peptides inside that digest were being dropped unreplaced.
  • AnUnblockedMissedCleavage_StillCountsWithTheFlagOn and FullSearch_TradesTheImpossibleFormForTheReadThrough — guard the opposite direction, so the fix cannot degrade into blanket zeroing or into dropping without replacing.

Plus the round-2 regressions: citrulline-on-R-only, Clone preserving the flag on both paths, a blocked residue at the protein C-terminus surviving, and a three-blocked-site read-through surviving at 0 missed cleavages.

Full offline suite: 5333 passed, 0 failed.

Note on CI: external-service-tests is red on this branch, inherited from master — it has been failing since 2026-07-30 on the commit that added GetProjectFilesFromFtp_LivePxd000001_IsMoreCompleteThanTheRestManifest (#1121). EBI's FTP is unreachable from GitHub runners, and because the walk returns an empty list rather than throwing, ExternalServiceTestHelper cannot classify it as an outage and it fails instead of skipping. Unrelated to this PR.

…olishing its cleavage site

Digestion cleaves first and places modifications afterwards, and nothing checked
whether a modification abolished the very site the peptide was cut at. So
digestion reported peptidoforms ending in an acylated lysine -- a cleavage
trypsin cannot perform -- frequently at MissedCleavages = 0. On human serum
albumin, 17 of 303 peptidoforms (5.6%) end in a modified C-terminal K; on a
heavily acetylated or succinylated substrate such as a histone the proportion is
far higher, because that is exactly where these modifications concentrate.

Trypsin's specificity comes from the positively charged K/R side chain binding
Asp189 in the S1 pocket. Acylating the lysine epsilon-amine removes that charge
(succinylation reverses it, adding a carboxylate), so the protease does not
cleave -- which is why acetylome and succinylome workflows are dominated by
missed cleavages at the modified residue.

Modification.BlocksCleavage now expresses that, backed by a curated classifier
(CleavageBlockingModifications). Putting it on the modification rather than
inside the digestion engine keeps the engine chemistry-agnostic and makes the
concept reusable; no modification database encodes "blocks protease cleavage",
so the set has to be curated regardless of where it lives. The methyl series is
deliberately excluded: methylation retains the charge and impairs rather than
abolishes cleavage, so it shows up as a missed cleavage, not an absent peptide.

Digestion consults it only when the new DigestionParams flag
RespectCleavageBlockingModifications is set. Default false reproduces the
historical, modification-blind digestion exactly -- important because mzLib CI
runs the MetaMorpheus suite and this ships downstream via NuGet.

Dropping the impossible peptidoform is not sufficient on its own. The REAL
peptide reads THROUGH the blocked residue to the next site, which costs a missed
cleavage under the ordinary count, so at MaxMissedCleavages = 0 it would never be
generated and the peptide would be lost entirely rather than merely mis-reported.
So a blocked residue stops counting as a missed cleavage: generation is given a
small slack, and an open-site filter then keeps only peptidoforms whose
C-terminus is a real cut and whose OPEN missed cleavages are within the caller's
limit. Every peptidoform is still produced exactly once -- no synthesis, no
deduplication.

Scoped deliberately, for review: the C-terminal drop is exact and applies to all
specificity modes; the read-through slack is applied to full-specificity
digestion only. A blocking modification on an internal residue is counted as
covering an internal site without re-deriving the protease's site list, so a
modified K or R that was never a site (trypsin's K|P rule) can discount a missed
cleavage it did not occupy; the count is clamped so it cannot go negative.

Full suite: 5338 passed, 0 failed, 29 skipped -- the previous 5334 plus the four
tests added here, so no existing digestion expectation moved.

Closes smith-chem-wisc#1113.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jp7uYMFdRCEWvg7sWZHFr2
@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.66667% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.53%. Comparing base (22441e1) to head (a30851e).

Files with missing lines Patch % Lines
...ics/Modifications/CleavageBlockingModifications.cs 75.00% 2 Missing and 4 partials ⚠️
...teomics/ProteolyticDigestion/ProteolyticPeptide.cs 87.87% 2 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1119      +/-   ##
==========================================
- Coverage   91.53%   91.53%   -0.01%     
==========================================
  Files         445      446       +1     
  Lines       53848    53916      +68     
  Branches     6570     6586      +16     
==========================================
+ Hits        49292    49350      +58     
- Misses       3277     3281       +4     
- Partials     1279     1285       +6     
Files with missing lines Coverage Δ
mzLib/Omics/Modifications/Modification.cs 97.09% <100.00%> (+0.01%) ⬆️
...Proteomics/ProteolyticDigestion/DigestionParams.cs 99.08% <100.00%> (+0.07%) ⬆️
...roteomics/ProteolyticDigestion/ProteinDigestion.cs 95.26% <100.00%> (+0.08%) ⬆️
...teomics/ProteolyticDigestion/ProteolyticPeptide.cs 94.20% <87.87%> (-5.80%) ⬇️
...ics/Modifications/CleavageBlockingModifications.cs 75.00% <75.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

/// filter in ProteolyticPeptide.GetModifiedPeptides, so only genuinely-reachable peptidoforms
/// survive; the cost is enumeration, not correctness.
/// </summary>
public const int CleavageBlockingReadThroughSlack = 2;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This concerns me. It means that if we are doing the new mod blocking missed cleavages, then the number of missed cleavages we get out of digestion may or may not match our digestion parameters. This feels bad.

Is this second property needed? An alternative approach would be to treat the mod blocking the cleavage, say acetyl lysine, as not a cleavage site and not considered for the missed cleavage count.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with Nic's assessment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm going to look into this but i it seems to me that a blocked cleavage is not a missed cleavage. It's not cleavable. we are reporting cleavages, not lysines.

/// enumerate by missed cleavage in the same way, and the C-terminal drop still applies to them.
/// </summary>
public int EffectiveMaxMissedCleavages =>
RespectCleavageBlockingModifications && SearchModeType == CleavageSpecificity.Full

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cleavage specificity == Full will prevent this from working in nonspecific and semi-specific searches, is that intended?

… cleavage-blocking change

Round-two review caught five genuine correctness bugs in the first commit; this
fixes all of them, plus the Arg chemistry and regression tests.

1. Classifier ignored LocationRestriction, so an N-terminal (alpha-amine)
   acetylation on K -- UniProt "N-acetyllysine", side chain still charged, NOT
   blocking -- matched contains("acetyl") + target K and was misclassified. Now
   only side-chain ("Anywhere.") modifications classify as blocking.

2. Clone() carried RespectCleavageBlockingModifications on only the None-specificity
   return path; every other mode's clone silently reverted to modification-blind
   digestion and compared unequal to its source. Both paths now pass it.

3. Digestion() read the budget off DigestionParams.EffectiveMaxMissedCleavages
   instead of the mutable MaximumMissedCleavages property that
   SpeedySemiSpecificDigestion also reads, so a caller mutating that property had
   the mutation silently discarded -- even flag-off. Removed EffectiveMaxMissedCleavages;
   the budget is now computed from MaximumMissedCleavages, so flag-off is byte-for-byte
   the historical path.

4. The read-through slack was a fixed 2, so three or more blocked sites in one
   peptidoform lost the real peptide entirely while the shorter forms were still
   dropped. Slack is now MaxMods: a peptidoform carries at most that many variable
   modifications, hence at most that many blocked sites, so every variable-mod
   read-through is reachable. (A fixed blocking modification is unbounded and remains
   a documented limitation.)

5. The C-terminal drop ignored specificity, firing on semi- and single-terminus
   peptides whose C-terminus is a length truncation rather than a protease cut. It
   is now gated on CleavageSpecificityForFdrCategory == Full, matching where the
   slack is applied; the "exact, all modes" claim was wrong and is corrected.

Also: arginine is now reachable and correct -- citrullination (deimination) removes
Arg's charge and blocks trypsin, so the previously-dead R branch classifies it.
The BlocksCleavage doc no longer claims a terminal acylated residue is an impossible
peptide end (it is a real one; position is digestion's call, not the modification's).

New regression tests pin each fix: citrulline-on-R-only, Clone preserves the flag on
both paths, a blocked residue at the protein C-terminus survives, semi non-Full
peptides are untouched by the flag, and a three-blocked-site read-through survives at
zero missed cleavages with slack = MaxMods. The residue and location gates are now
exercised by a blocking-name-on-non-cleavage-residue case and an N-terminal-K case.
"Reproduces historical behaviour" now pins concrete pre-PR output and that the
impossible form is kept, rather than comparing two flag-off runs.

Deferred (documented, not fixed here): the surviving read-through still reports the
modification-blind MissedCleavages count; truncation-product ends beyond the protein
C-terminus are not exempted; BlocksCleavage recomputes on each access. Raised in the
PR thread for a follow-up.

Full suite: 5343 passed, 0 failed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jp7uYMFdRCEWvg7sWZHFr2
@trishorts

Copy link
Copy Markdown
Contributor Author

Follow-up commit 02bce317 — round-2 review caught five real correctness bugs; all fixed

A second /review-pr pass returned Request changes with 8 Major findings, and it was right — five were genuine correctness bugs in the first commit, not test nits. Writing them up plainly:

  1. N-terminal α-amine acetylation on K was misclassified as blocking. The classifier decided from target motif + name substring and never looked at LocationRestriction. UniProt ships both N6-acetyllysine (ε-amine side chain, genuinely blocking, Anywhere.) and N-acetyllysine (α-amine, N-terminal.) — the terminal form leaves the side chain charged, so trypsin still cleaves, yet both matched. Fix: only side-chain (Anywhere.) modifications classify.

  2. Clone() dropped the flag on the non-None path. The flag was passed on only the None-specificity return, so a cloned DigestionParams silently reverted to modification-blind digestion for every other mode (and compared unequal to its source). Fix: both paths pass it.

  3. Digestion() bypassed the mutable missed-cleavage property. It read EffectiveMaxMissedCleavages off DigestionParams instead of the MaximumMissedCleavages instance property that SpeedySemiSpecificDigestion also reads, so a caller mutating that property had the mutation silently discarded — even flag-off. Fix: removed EffectiveMaxMissedCleavages; the budget is computed from MaximumMissedCleavages, so flag-off is byte-for-byte the historical path.

  4. Slack of 2 lost real peptides when >2 sites were blocked. With three blocked K's in a row, the read-through spanning them was never generated while the shorter forms were still dropped — the peptide vanished. Fix: slack is now MaxMods (a peptidoform carries at most that many variable mods, hence blocked sites, so every variable-mod read-through is reachable). Fixed blocking mods remain a documented unbounded edge.

  5. The C-terminal drop fired on non-cut termini. It ignored specificity, so semi- and single-terminus peptides whose C-terminus is a length truncation were wrongly dropped — my "exact, all modes" claim was false. Fix: gated on CleavageSpecificityForFdrCategory == Full, matching where the slack applies.

Also corrected: arginine is now reachable and right — the previously-dead R branch classifies citrullination (deimination removes Arg's charge → trypsin doesn't cleave); and the BlocksCleavage doc no longer claims a terminal acylated residue is an impossible peptide end (it's a real one — position is digestion's call).

New regression tests pin each fix: citrulline-on-R-only, Clone preserves the flag on both paths, a blocked residue at the protein C-terminus survives, semi non-Full peptides are untouched by the flag, and a three-blocked-site read-through survives at 0 missed cleavages with slack = MaxMods. The residue and location gates are now exercised (a blocking name on a non-cleavage residue; an N-terminal-K), and "reproduces historical behaviour" pins concrete pre-PR output plus that the impossible form is kept — instead of comparing two flag-off runs.

Deferred, and raised here honestly for a follow-up (all Minor):

  • The surviving read-through still reports the modification-blind MissedCleavages count (the blocked site "isn't a cut" but the stored count doesn't reflect that). Recomputing it per-peptidoform is a deeper change.
  • Truncation-product ends beyond the protein C-terminus aren't exempted from the drop.
  • BlocksCleavage recomputes (lowercases + scans the name) on each access rather than caching.
  • The name-keyed acyl-stem classifier is inherently a heuristic; broader Unimod coverage and the contains("methyl") bluntness are curation refinements. The persisted-custom-.txt-field path (BM/BL precedent) remains the upgrade for user-declarable blocking mods.

Full suite: 5343 passed, 0 failed.

This is exactly why the second review pass is worth running — same as #1102, the round-1 hardening had bugs a first review of the original PR could not have seen.

Review concern from nbollis, seconded by Alexander-Sol: with the flag on, the
missed cleavage count coming out of digestion did not have to match the
digestion parameters going in. Confirmed -- at maxMissedCleavages: 0 the
emitted peptides reported < 0, 0, 1 >.

Cause: generation inflates the span by MaxMods so the read-through form of a
blocked site is reachable, and the open-site filter then decided whether a
peptidoform survived using the blocked-discounted count -- but the peptide was
still CONSTRUCTED with the modification-blind count. The slack leaked out as
peptides claiming more missed cleavages than the caller asked for.

The discounted count is now what the surviving peptidoform carries. This is
the reviewers' own proposed alternative, and it is the semantics the flag
already claimed: a blocked residue is not a cleavage site for the peptidoform
carrying it, so it is not a missed cleavage either. The count reports cleavages
that could have happened and did not, not Lys/Arg residues. The slack stays,
but is now purely an enumeration detail and is invariably discounted before a
peptide is emitted.

Three tests. Two verified red against the previous source: the read-through of
a blocked site now reports 0 rather than 1, and no emitted peptide at any
budget (0/1/2) reports more missed cleavages than requested. The third guards
the other direction -- an ordinary open K is still counted -- so the fix cannot
degrade into blanket zeroing.

Also documents the answer to the second review question (the CleavageSpecificity
Full gate is intended for now, and semi peptides whose C-terminus is a genuine
cut are knowingly not yet filtered), and drops a see-cref to
CleavageBlockingReadThroughSlack, deleted in 02bce31.

Offline suite: 5330 passed, 0 failed.
…rches

Tested at nbollis's prompting about the CleavageSpecificity gate, and the two
halves of the feature were gated on different things: the C-terminal drop on
the PEPTIDE's specificity, the generation slack on the PARAMS' SearchModeType.
A semi search satisfied the first and not the second, so it got half the trade.

Measured on AAAAAARPEPTKPEPTRAAAAAAK with an acetyl-K, flag off vs on:

  Full mode, maxMC=0:  -PEPTK[Acetyl]  +PEPTK[Acetyl]PEPTR   6 -> 6
  Semi mode, maxMC=0:  -PEPTK[Acetyl]  (nothing added)       6 -> 5

The impossible peptidoform left and the read-through that should replace it was
never enumerated, because no slack is granted outside a full search. That makes
the peptide unidentifiable rather than correctly identified -- strictly worse
than leaving it alone. It bites whenever the read-through needs more budget than
maxMissedCleavages allows: one blocked site at 0, three consecutive at 2.

The drop now also requires SearchModeType == Full, so a semi or nonspecific
search is untouched, which is what the documentation already claimed. Making
semi benefit properly is follow-up and is not a gate widening: a semi peptide's
C-terminus may be a real cut or a length truncation, and separating them needs
the protease site list that full digestion gets free from its enumeration.

SemiSpecific_NonFullPeptides_AreUnaffectedByTheFlag compared only the non-Full
subset of a semi digest, which is exactly why it stayed green while the Full
peptides inside that digest were dropped unreplaced. It now asserts the whole
digest at three budgets, and fails on all three without the new gate. A paired
test states the Full-mode contract as the exchange it is.

Offline suite: 5333 passed, 0 failed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants