Skip to content

Truncation search: follow-on task for N-/C-terminal proteoform truncations#2666

Open
trishorts wants to merge 44 commits into
smith-chem-wisc:masterfrom
trishorts:truncation-search-mm
Open

Truncation search: follow-on task for N-/C-terminal proteoform truncations#2666
trishorts wants to merge 44 commits into
smith-chem-wisc:masterfrom
trishorts:truncation-search-mm

Conversation

@trishorts

Copy link
Copy Markdown
Contributor

Truncation search: a follow-on task for proteoform N-/C-terminal truncations

Summary

Adds a new TruncationSearchTask that runs after a normal top-down search and discovers N- and C-terminally truncated proteoforms of the parents identified in that search. It is a standard MetaMorpheus task (run-list [Search, Truncation], dispatchable from CMD) with its own TOML schema and per-class FDR, and it writes results using the existing PSM/proteoform .psmtsv columns.

The task is self-contained and depends on no unreleased code — it builds against the current released mzLib and uses only standard MetaMorpheus/mzLib APIs (digestion, fragmentation, MatchFragmentIons/CalculatePeptideScore, FdrAnalysisEngine, the standard PSM writer). It works on either classic deconvolution or an external precursor source; nothing in the truncation code requires a particular precursor pipeline.

How it works (three passes)

  1. Parents from the upstream search. The Search task deposits its resolved, FDR'd proteoform-level PSMs into a small TaskChainContext (in-memory hand-off; a disk AllProteoforms.psmtsv fallback also exists). The truncation task filters those to confident parents and expands pipe-ambiguous identifications into individual parents, then adds reverse decoys for any parent lacking one.
  2. Pass 2 — candidate location. A fragment-bin index (N- and C-series of every parent) plus a truncation-aware mass acceptor locate candidate parents per scan; each candidate is scored twice with the standard scorer restricted to the N-series and to the C-series, and the highest (parent, series) wins. Scans already explained intact are skipped.
  3. Pass 3 — chopping + scoring. The terminus opposite the winning series is chopped one-or-more residues until the truncated mass matches at an allowed notch; surviving mods are re-indexed onto the shorter form (with terminal-mod retention), both series are scored with the standard scorer, and duplicate truncations collapse into one ambiguous PSM. The truncation type (N-/C-terminal truncation(start-end)) rides the existing Description column, mirroring the existing chain(2-121) descriptors.

Intact PSMs are inherited as full-length, then everything is pooled into one per-class FDR/PEP pass and written to AllTruncatedPSMs.psmtsv / AllTruncatedProteoforms.psmtsv.

Additional pieces in this PR:

  • SequenceTagExtractor / ProteinTagIndex — optional sequence-tag candidate restriction (tags emitted only for unambiguous residue gaps).
  • InternalTruncationSearch / FragmentationPropensity — opt-in internal (both-ends) fragment search with a separate per-class FDR, plus a fragmentation-propensity scoring foundation.
  • PerfLogger — opt-in perf_log.tsv metrics/timings (null path = no-op), for benchmarking.

Tests

66 NUnit tests covering the acceptor, the Pass 2 engine (dual-series scoring, parent filtering, oversized-parent handling, pipe-split, q-filter), the chopper (C/N chop, mod retention, collapse, notch), reverse-decoy generation, pooled FDR + output columns, sequence-tag extraction, fragmentation propensity, TOML round-trip / CMD dispatch, and an end-to-end [Search, Truncation] run that asserts well-formed output. Coverage on EngineLayer.Truncation.* is ~97% line. All 66 pass on the current master base.

Notes

  • New MyTask.Truncation enum member; CMD/Program.cs dispatches Toml.ReadFile<TruncationSearchTask>.
  • TaskChainContext is a generic deposit/retrieve slot assigned by EverythingRunnerEngine to every task before RunTask; the Search task's deposit is a no-op when no context is present.
  • Also removes a stray committed VS hot-reload temp file (GUI_h05aw1sq_wpftmp.csproj).

🤖 Generated with Claude Code

trishorts and others added 30 commits May 26, 2026 08:48
GUI_h05aw1sq_wpftmp.csproj is a Visual Studio WPF hot-reload temp file
accidentally committed upstream (smith-chem-wisc#2627). It isn't part of MetaMorpheus.sln,
VS regenerates it on demand, and it still pinned a stale mzLib 1.0.574 — pure
confusion. Remove it from the project branch.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Introduces the top-down TruncationSearchTask as a MetaMorpheusTask subclass whose RunSpecific is an empty stub, plus EngineLayer/Truncation stubs (TruncationAcceptor, TruncationSearchEngine, ProteoformChopper) that the Phase 1-3 algorithm will fill in.

Adds an in-memory TaskChainContext so a finishing task can hand results to a later one (SearchTask -> TruncationSearchTask) without a file round-trip: EverythingRunnerEngine assigns one shared context to every task and MetaMorpheusTask exposes it as a [TomlIgnore] slot. Registers the MyTask.Truncation enum member.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
NUnit 4 fixture asserting TruncationSearchTask default parameters, TaskChainContext deposit/retrieve semantics, and an end-to-end [SearchTask, TruncationSearchTask] EverythingRunnerEngine run on the existing tiny top-down fixture that completes without throwing and leaves the truncation task with an empty output folder.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…eries scoring

TruncationAcceptor accepts a parent for an observed precursor iff 0 < M_obs <= M_theo + tolerance (single notch), so only parents at least as heavy as the scan are truncation candidates.

TruncationSearchEngine builds a fragment index over both ion series of every parent (ModernSearch binning), skips scans Pass 1 already matched intact, gathers candidates via the index and acceptor, then scores each candidate twice -- N-terminal-ion series only and C-terminal-ion series only -- with the standard matched-ion score; the highest (parent, series) pair wins. Opposing-series ions are never matched in a direction, so they neither disqualify nor inflate a candidate. Parents over MaxFragmentSize are excluded with one summary warning.

TruncationParentBuilder turns deduped Pass 1 rows into the combined parent list: permissive q-value filter (PEP q else notch q) and pipe-ambiguous expansion, each alternative tied back to its originating row.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…p utility

Synthetic in-memory fixture (three top-down parents, eight scans) exercises intact-skip, C-/N-truncation series selection, the precursor acceptor's reject cases, opposing-series tolerance, and the MaxFragmentSize exclusion; plus pipe-ambiguous expansion and the PEP-vs-notch q-value filter. A smoke test exercises the mzLib ExportSnipAsMzML utility on Windows. Coverage on EngineLayer.Truncation is line 96.75% / branch 92.18% / method 96.77%.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ProteoformChopper.ChopUntilMassMatches removes one residue at a time from the indicated terminus -- dropping any PTM locked to that residue and the terminal mod on it, so an N-terminal acetyl-Ala leaves in a single step -- until the truncated mass matches the precursor at an allowed notch, re-indexing surviving mods onto the shorter form and reporting protein start/end coordinates.

TruncationPass3 chops the terminus opposite the winning ion series, scores the deduced truncated form against the scan with standard both-series MetaMorpheus scoring, emits a SpectralMatch labeled N-/C-terminal truncation, and collapses duplicate truncations (same truncated sequence + scan) into one PSM with pipe-merged parent accessions.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Eleven tests over a modified-parent fixture: C-/N-terminal chop with correct protein coordinates, phospho retention through an N-terminal chop, N-acetyl-Ala removed as one step, no-match returns null, +1 isotope notch acceptance, two parents collapsing to one PSM with pipe-separated accessions, Pass 3 score equal to both-series CalculatePeptideScore, terminal-mod retention when the opposite terminus is chopped, and the N-terminal-truncation label. Coverage on EngineLayer.Truncation is line 97.41% / branch 94.44% / method 95.74%.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…olumn

Sets each truncated peptide's PeptideDescription to 'N-/C-terminal truncation(<start>-<end>)', which the standard psmtsv writer renders verbatim in the existing Description column (alongside proteolysis-product descriptors like chain(2-121)). Avoids adding a new column or a custom writer; resolves the 01_Architecture.md #13 assumption that a standalone Truncation Product Type column already existed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…1 counterpart

AddReverseDecoys augments the combined parent list with a fresh reverse decoy for every target whose DECOY_<accession> is not already present, reusing mzLib's GetReverseDecoyFromTarget so PTMs lock to residues. Targets keep any decoys inherited from Pass 1 (decision #14).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Asserts PEPT(phospho)IDE reverses to EDIT(phospho)PEP with the phospho still on the T, and that a target already paired with a Pass 1 decoy gets no duplicate.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
RunPooledFdr resolves ambiguities, drops duplicate PSMs per scan, then runs the standard FdrAnalysisEngine (PEP trains inside when the pool is large enough; N- and C-terminal truncations share one FDR category, #15/#18).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…us PSM

CollapseDuplicateTruncations now merges same-(scan, truncated sequence) hits into a single SpectralMatch via AddOrReplace, so the standard writer pipe-merges parent accessions natively on ResolveAllAmbiguities (#10) instead of carrying a hand-built accession string.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
WritePsms/WriteProteoforms emit AllTruncatedPSMs/Proteoforms with the standard SpectralMatch header and rows (truncation type in the Description column, #13); no q-value/PEP cutoff at write time, decoys and contaminants included (#16/#17). Proteoforms dedupe by truncated FullSequence.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pooled-FDR q-value monotonicity and small-pool PEP skip; output column set, one-row-per-PSM / per-proteoform counts, full-length label, and pipe-merged accessions for collapsed parents.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After post-search analysis, hand the resolved, FDR'd SpectralMatch list to
the shared in-memory TaskChainContext keyed by the task id (decision #1), so a
downstream TruncationSearchTask can seed Pass 2 from it without a file round
trip. No-op when no context or consumer is present.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Ingest the upstream proteoforms (in-memory context, AllProteoforms.psmtsv disk
fallback), dedupe to proteoform level, apply the permissive parent q-filter
(#3), expand pipe-ambiguity into parents (#2), add reverse decoys (#14), then
per file run Pass 2 dual single-series scoring, chop winners (Pass 3), inherit
intact matches as full-length forms (#4a, new TruncationPass3.InheritAsFullLength),
pool, run pooled FDR/PEP (#15/#18), and write the two result files (#16/#17).

Resolve the dissociation type per scan when CommonParameters is Autodetect
(mirroring ClassicSearchEngine) and build the fragment index over every type
the scans use, so fragmentation no longer fails on Autodetect top-down data.
TruncationSearchParameters gains MassDiffAcceptorType/CustomMdac for the Pass 3
chop notches (smith-chem-wisc#80).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Run [SearchTask, TruncationSearchTask] through EverythingRunnerEngine on the
sliced yeast top-down fixture, then assert both AllTruncated*.psmtsv files are
written with the standard psmtsv header and at least one pooled row, including
the intact match inherited as a full-length form (#4a). The previous fixture
(Jurkat/histone) produced no search hits, so it never exercised the pipeline.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add the Truncation case to the CMD task switch so a run-list TOML (TaskType = "Truncation") deserializes and runs. When no UpstreamSearchTaskId is configured, the task now ingests the most recently deposited proteoform set from the TaskChainContext (new TryGetMostRecent), so a [Search, Truncation] CMD run needs no exact-id wiring.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Assert a TruncationSearchTask round-trips through TOML (TaskType = "Truncation" plus the truncation-specific settings) so CMD can read and dispatch it, and that TaskChainContext.TryGetMostRecent returns the latest assignable deposit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When TruncationSearchParameters.PerfLogPath is set, the task appends one row to a rolling, append-only perf_log.tsv (03_Benchmarks schema): per-pass timings (pass2 index/scoring exposed from the engine, pass3 chop/score via a TruncationTimings accumulator, FDR+PEP), truncation/full-length/parent/decoy counts, q01/q05 yields, peak memory, and phase/dataset/run-label parsed from the run-folder name. Null path (default) is a no-op, so normal runs and tests are unaffected. git commit/dirty are best-effort.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… row

Unit tests for PerfLogger.Append (header-once + append) and ParseRunFolderName; the [Search, Truncation] e2e now sets PerfLogPath and asserts one TruncationSearchTask row whose n_psms_emitted matches the PSM TSV row count.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Use ProcessStartInfo.WorkingDirectory instead of git -C "<dir>": AppContext.BaseDirectory ends in a separator, so the trailing backslash before the closing quote escaped it and broke git's arg parsing, leaving git_commit empty. Verified the perf log now records the branch HEAD.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pass 2 re-fragmented every candidate parent per scan and kept any parent with one matching fragment bin -- fine for a few hundred observed parents, catastrophic for a database-wide (millions) parent set. Rebuild it the way the core search scales: two series-tagged fragment indexes (N: b/c, C: y/z), then per scan a single pass over the experimental peaks counts each parent's matched bins per series; parents below the count that could possibly clear ScoreCutoff are pruned (provably loss-free at the default cutoff), and only the top MaxCandidatesToScore are authoritatively rescored with the standard both-series matched-ion score. Index build (per parent) and scoring (per scan) both run in parallel. Adds CandidateRank/CandidatePoolSize diagnostics. 50 truncation tests green (the S1-S8 winner-selection tests, written against the brute-force scorer, confirm the indexed path picks the same winners).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
SeedParentsFromDatabase (off by default) generates theoretical full-length parents from the protein DB using the same Protein.Digest path the intact top-down search uses (ClassicSearchEngine), so XML-annotated PTMs and the configured variable mods ride along -- removing the requirement that a truncation's parent be identified intact. MaxParentMass makes the engine's parent-mass cap configurable (default 30 kDa); raise it so large proteins whose small fragments populate an LMW fraction can be chopped. Writes CandidateRanks.tsv (per-scan winner rank). Pilot (R1, bounded mods, 60 kDa): tractable in ~19 min on the indexed engine; 51% of FDR-surviving truncations come from proteins never identified intact.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…inal truncation

Decision #13: a clean removal of exactly the initiator Met (1 residue, parent starts at protein pos 1 with M) is the canonical NME form, not a 1-residue N-terminal truncation. Label it 'N-terminal Met excision' (or '... + acetylation' when the new N-terminus carries an acetyl) so NME is not miscounted as truncation -- Chen 2019 found ~17% of reported truncations were plain NME, ~13% NME+acetyl, so our N-terminal truncation counts were inflated. ProteoformChopper.ClassifyChop sets the label; ScoreTruncation reads it back from the Description so the product-type column stays consistent. +4 tests; 54 truncation tests green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
First step of the doc's principled parent-free mechanism (§11.2.4, pTop-style §3.4.4): SequenceTagExtractor derives short I/L-normalized de-novo tags from a scan's fragment-mass gaps (recall-oriented; ambiguous high-mass gaps over-generate so the true protein is not missed), and ProteinTagIndex builds a parallel k-mer index over the database that turns those tags into a small candidate-protein set. These let a database-seeded truncation search restrict candidates per scan instead of materializing all-DB proteoforms (the 397 GB blowup) and competing every scan against millions (the winner-competition loss). Foundation only -- integration into a tag-filtered BuildParentsFromDatabase path is next. +6 tests; 60 truncation/tag tests green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…B-seeding

Wire SequenceTagExtractor + ProteinTagIndex into a tag-filtered BuildParentsFromDatabase path (UseSequenceTagFilter): extract de-novo tags from every scan, take the union of tag-supported proteins, and digest only those into parents instead of the whole DB. Hoists scan loading ahead of parent building (so the filter sees all scans; engine loop reuses the loaded scans). Tag extraction (per scan) and digestion (per candidate protein) are parallel. Global-union variant controls the all-DB parent explosion (the 397 GB blowup) but does not yet restrict candidates per scan -- that (and the winner-competition fix) is the next per-scan step. Logs the selected/total protein count. 60 truncation/tag tests green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
UsePerScanTagRestriction: with the tag filter on, score each scan only against parents whose protein its own de-novo tags supported, rather than the global union. The tag-filtered builder now also returns a per-scan allowed-accession map; the engine takes an optional map and, during candidate gathering, skips parents whose underlying (DECOY-stripped, so target+decoy stay balanced for FDR) accession is not in the scan's set. Fixes the winner-competition loss (a scan can no longer be won by a protein its tags did not support) and gives per-scan selectivity. Off by default; requires UseSequenceTagFilter. +1 engine test (disallowed protein -> no winner; allowed -> wins); 61 truncation/tag tests green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…n (C-Score core)

Borrowable core of ProSightPD's C-Score (LeDuc 2014, verified from the open paper): weight each matched fragment ion by the cleavage frequency of the two residues flanking the bond it reports, so matches at chemically-favored bonds (C-terminal to D/E, N-terminal to P) count more than coincidental matches -- sharpening true-vs-false separation, which is what the internal-fragment class needs. First-pass static HCD/CID weight table; uniform weights reduce exactly to the standard matched-ion score. Includes the fragment-ion->backbone-bond mapping. +3 tests. Next: re-add internal chopping, score the internal class with this + a strong bilateral gate, and run it under its OWN (per-class) FDR.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reinstates ProteoformChopper.ChopInternalCandidates (enumerate mass-matched internal spans of a parent, monotone early-break) and the 'internal truncation' label/classification on the current indexed-engine branch (the original internal work was stashed to internal-truncation-parked when the branch was reset). Foundation for the C-Score-style internal class. +1 test; 65 truncation/tag/propensity tests green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
trishorts and others added 3 commits May 26, 2026 08:48
…ricted)

InternalTruncationSearch: for each scan, enumerate internal spans of each parent whose mass matches the precursor -- found by prefix-sum arithmetic + binary search so only mass-matches are built (O(L log L) per scan-parent, not O(L^2) object builds) -- score each candidate's OWN b/y ladder with the propensity-weighted score, and keep the best clearing a strong bilateral gate (>=K matched ions on EACH new terminus). This is the correct design (an internal fragment shares no ladder with its parent, so Pass 2's parent index cannot find it -- why the parked chop-the-winner approach got ~0). Parallel over scans; parent prefix sums precomputed once. Adds ProteoformChopper.BuildTruncatedForm. +1 test; 66 green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…per-class FDR

SearchInternalTruncations runs InternalTruncationSearch over the parent set per file, collapses duplicates, and judges the internal PSMs in their OWN target-decoy FDR (the per-class normalization), writing AllInternalTruncations.psmtsv -- independent of the terminal/intact pool and its threshold. InternalMinIonsPerTerminus sets the bilateral gate. Off by default; 66 tests green (regression clean).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Tighten the de-novo tag extractor: emit an edge only when a fragment-mass gap matches EXACTLY ONE residue within the window (ambiguous gaps, e.g. K vs Q at high mass, are dropped rather than guessed). With the top-N-intensity peak input, this cut the global-union tag selection from 99.9% of the DB to ~22% (4522/20429). Was built/tested with the tag-filter work but not separately committed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented May 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.33333% with 73 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.30%. Comparing base (3c8e0b8) to head (bc35c46).

Files with missing lines Patch % Lines
...Layer/TruncationSearchTask/TruncationSearchTask.cs 88.35% 15 Missing and 19 partials ⚠️
...s/EngineLayer/Truncation/TruncationSearchEngine.cs 95.57% 6 Missing and 6 partials ⚠️
MetaMorpheus/TaskLayer/PerfLogger.cs 89.18% 3 Missing and 5 partials ⚠️
...Morpheus/EngineLayer/Truncation/TruncationPass3.cs 93.10% 0 Missing and 4 partials ⚠️
MetaMorpheus/CMD/Program.cs 0.00% 3 Missing ⚠️
...Morpheus/EngineLayer/Truncation/ProteinTagIndex.cs 94.33% 2 Missing and 1 partial ⚠️
...EngineLayer/Truncation/InternalTruncationSearch.cs 97.91% 0 Missing and 2 partials ⚠️
...rpheus/EngineLayer/Truncation/ProteoformChopper.cs 96.96% 1 Missing and 1 partial ⚠️
...eus/EngineLayer/Truncation/SequenceTagExtractor.cs 95.65% 0 Missing and 2 partials ⚠️
.../EngineLayer/Truncation/FragmentationPropensity.cs 95.00% 0 Missing and 1 partial ⚠️
... and 2 more
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #2666     +/-   ##
=========================================
  Coverage   93.29%   93.30%             
=========================================
  Files         196      212     +16     
  Lines       21320    22414   +1094     
  Branches     3994     4210    +216     
=========================================
+ Hits        19891    20913   +1022     
- Misses        893      923     +30     
- Partials      536      578     +42     
Flag Coverage Δ
unittests 93.30% <93.33%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...pheus/EngineLayer/Truncation/TruncationAcceptor.cs 100.00% <100.00%> (ø)
...taMorpheus/EngineLayer/Truncation/TruncationFdr.cs 100.00% <100.00%> (ø)
...orpheus/EngineLayer/Truncation/TruncationOutput.cs 100.00% <100.00%> (ø)
.../EngineLayer/Truncation/TruncationParentBuilder.cs 100.00% <100.00%> (ø)
...rpheus/EngineLayer/Truncation/TruncationTimings.cs 100.00% <100.00%> (ø)
MetaMorpheus/TaskLayer/EverythingRunnerEngine.cs 71.60% <100.00%> (+0.71%) ⬆️
MetaMorpheus/TaskLayer/MetaMorpheusTask.cs 86.39% <100.00%> (+0.01%) ⬆️
...TruncationSearchTask/TruncationSearchParameters.cs 100.00% <100.00%> (ø)
.../EngineLayer/Truncation/FragmentationPropensity.cs 95.00% <95.00%> (ø)
MetaMorpheus/TaskLayer/SearchTask/SearchTask.cs 95.73% <66.66%> (-0.29%) ⬇️
... and 10 more

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@trishorts

Copy link
Copy Markdown
Contributor Author

Manual unit-test review

Every unit test this PR adds was reviewed individually by the author, rather than relied on solely as automated/AI-generated checks. Of the 65 tests added:

  • 55 were approved as written;
  • 8 received explanatory doc-comments (the Met-excision / internal-fragment cases in TruncationChopperTests);
  • 1 was strengthened (Acceptor_ProseString_DescribesRule now asserts the full acceptance rule, not just that the prose mentions "observed");
  • 1 was removed as a tautology (Scaffolding_TestProjectWired, a leftover Assert.True placeholder that tested nothing) — leaving 64 retained.

This satisfies the author's standing AI-use policy that AI-assisted tests receive human review before being relied upon. Recorded 2026-06-22.

trishorts and others added 7 commits June 22, 2026 12:39
Outcome of a manual, per-test review of the unit tests this PR adds
(human review of AI-assisted tests):

- TruncationSearchEngineTests: Acceptor_ProseString_DescribesRule now
  asserts the full acceptance rule (observed / theoretical / "0 <" / "<="),
  not merely that the prose mentions "observed".
- TruncationSearchTaskTests: remove Scaffolding_TestProjectWired, a
  tautological Assert.True placeholder that verified nothing and inflated
  apparent coverage.
- TruncationChopperTests: add explanatory doc-comments to the Met-excision
  and internal-fragment cases.

No other assertion logic changed.

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

AddReverseDecoys keyed decoys on the protein accession, so a protein with
several identified proteoforms (e.g. unmodified + oxidized, or pipe-ambiguity
alternatives) got only one decoy and the extra target proteoforms had no decoy
counterpart -- under-counting decoys and biasing the pooled target-decoy FDR
anti-conservatively. Key generation/de-dup on the distinct proteoform (decoy
accession + reversed full sequence) so each target proteoform gets its own
decoy, while still suppressing decoys already inherited from Pass 1.

Also skip rows with null/empty FullSequence before Split('|') (no more NRE).

Adds ReverseDecoy_OneDecoyPerProteoform_NotPerAccession (two A123 proteoforms
-> two decoys, one carrying the reversed oxidation).

Addresses findings #6 and #10 from the PR smith-chem-wisc#2666 review.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DWZ1KbGenS4w1beTMXWgek
- ProteinTagIndex.GetCandidateProteinIds: count DISTINCT query tags so a
  repeated tag can't inflate a protein toward minTagHits.
- PerfLogger: sanitize string cells (strip tab/CR/LF) so a stray character
  can't shift TSV columns; drain stderr in RunGit so a chatty git can't fill
  the pipe buffer and block.
- TaskChainContext.Deposit: refresh recency on re-deposit so TryGetMostRecent
  can't return a stale entry.

Addresses findings #7, #4, #5, #3 from the PR smith-chem-wisc#2666 review.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DWZ1KbGenS4w1beTMXWgek
- SearchTask: null-filter the proteoform set at the task-chain deposit site so
  the "no nulls" contract is enforced locally, not via a PostSearchAnalysisTask
  side effect (#1).
- TruncationSearchTask: Warn when UsePerScanTagRestriction is set without its
  SeedParentsFromDatabase + UseSequenceTagFilter prerequisites, instead of
  silently doing nothing (#12).
- InternalTruncationSearch: document that NotchSearchWindow is a fixed constant
  not derived from the acceptor, and when to widen it (#8).
- Tests: wrap temp-file/dir cleanup in try/finally so artifacts don't leak on
  assertion failure (#16, #19).

Addresses findings #1, #8, #12, #16, #19 from the PR smith-chem-wisc#2666 review.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DWZ1KbGenS4w1beTMXWgek
- TruncationSearchTask: write CandidateRanks.tsv only when PerfLogPath is set,
  register it via FinishedWritingFile, and wrap it best-effort so a diagnostic
  write can't abort the task (#14). Document the intentional PEP==0 disk-filter
  guard and the in-memory-only PepQWasUsed metric (#11, #15 -- wontfix).
- TruncationSearchEngineTests: ScanS7 now asserts the opposing-series peaks
  neither raise the score nor change the integer matched-ion count, so the test
  distinguishes "ignored" from merely "harmless" (#17).
- TruncationFdrAndOutputTests: PepRuns pins the skipped-PEP branch (PEP_QValue
  == 2 on a too-small pool) rather than only asserting no throw (#18).

Addresses findings #14, #17, #18 and documents #11/#15 from the PR smith-chem-wisc#2666 review.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DWZ1KbGenS4w1beTMXWgek
Lift patch coverage above 90% on the new files codecov flagged:
- TruncationSearchTask 52% -> 92%: three standalone-task integration tests
  exercise the database-seeded, sequence-tag-filtered, and disk-ingest parent
  paths (plus perf-log/CandidateRanks output and the internal-fragment search).
- PerfLogger 92% -> 94%: ParseRunFolderName empty/null defaults, plus a test
  that a tab/newline in a string cell cannot shift TSV columns.
- TruncationPass3 96% -> 100%: ScoreTruncation timing-instrumentation branch.

Remaining uncovered lines are git-subprocess and diagnostic-write error
paths that can't be triggered deterministically.

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

@pcruzparri pcruzparri left a comment

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.

K-LLM ensemble code review — 3 finding(s) at severity >= High, posted as inline comments.

All three share the same root cause: a single List<Product> is created outside a candidate loop and passed to PeptideWithSetModifications.Fragment(...), which appends without clearing, so MatchFragmentIons scores each candidate against an ever-growing union of fragments from all prior candidates. This inflates matched-ion counts and biases winner selection.

Method: 9 independent open-source LLM reviewers (Big Pickle, DeepSeek V4 Flash/Pro, MiMo-V2.5, Nemotron 3 Ultra, North Mini Code, Qwen 3.5/3.6 Plus, Kimi K2.6) each analyzed a differently-shuffled copy of this PR's diff. Findings were clustered by (file region, root cause), majority-voted, then validated against the PR branch source. 8/9 reviewers returned results; 3 findings survived validation.

All three are in code added by this PR. One reviewer (nemotron-3-ultra-free) returned empty output on two attempts; it is excluded from the vote denominator.

Comment thread MetaMorpheus/EngineLayer/Truncation/InternalTruncationSearch.cs
Comment thread MetaMorpheus/EngineLayer/Truncation/TruncationSearchEngine.cs
Comment thread MetaMorpheus/EngineLayer/Truncation/TruncationSearchEngine.cs
trishorts and others added 2 commits June 29, 2026 08:48
A single List<Product> was allocated outside the candidate loop and
passed to Fragment(), which appends without clearing. MatchFragmentIons
then scored each candidate against the accumulated union of fragments
from all prior candidates, inflating matched-ion counts and biasing
winner selection. Clear the list before each Fragment() call at three
sites: the Pass-2 rescoring loop (nTermProducts/cTermProducts), the
fragment-index build (products), and the internal-truncation span loop.

Addresses the three findings in the PR smith-chem-wisc#2666 review.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@trishorts

Copy link
Copy Markdown
Contributor Author

Updated the branch and addressed the review:

  • Merged current smith-chem-wisc/master into the branch (now current with master).
  • Fixed all three K-LLM ensemble findings (the shared List<Product> accumulation root cause) in 3aafb51b: cleared the reused fragment list before each Fragment(...) call at the three sites — internal-span loop, Pass-2 N/C rescoring, and the fragment-index build. Per-comment replies inline.
  • Truncation test suite green locally (76/76, Release/x64).

@trishorts

Copy link
Copy Markdown
Contributor Author

Correction / follow-up on the K-LLM review: after verifying against the source and at runtime, all three findings are false positives, and I've reverted my earlier change (a9b35ac3).

The findings assumed PeptideWithSetModifications.Fragment(...) appends to the passed list. It does not — it calls products.Clear(); as its first line (mzLib PeptideWithSetModifications.cs:240), in the perf-critical path that is deliberately called once per candidate. Reusing a single List<Product> across candidates is therefore the intended, memory-efficient idiom; there is no cross-candidate accumulation and no score inflation. MatchFragmentIons is always invoked immediately after Fragment(...), so it only sees the current candidate's products.

Runtime check (mzLib 1.0.579): fragment parent A into a list → 79 products; fragment parent B into the same list → 29 (B's own count), not 108.

Net state of the branch: synced to current smith-chem-wisc/master; the three engine files are unchanged from their reviewed form; truncation test suite green locally (76/76, Release/x64). Marking the three threads resolved.

@trishorts trishorts requested a review from pcruzparri June 29, 2026 14:38
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.

2 participants