Skip to content

Sequence-variant test corpus — an executable spec for variant × modification handling (L0)#1084

Merged
trishorts merged 7 commits into
smith-chem-wisc:masterfrom
trishorts:variant-corpus
Jul 10, 2026
Merged

Sequence-variant test corpus — an executable spec for variant × modification handling (L0)#1084
trishorts merged 7 commits into
smith-chem-wisc:masterfrom
trishorts:variant-corpus

Conversation

@trishorts

@trishorts trishorts commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

The nut

Handling proteins that carry sequence variants and modifications at the same time is one of mzLib's most bug-prone corners: a variant can delete a modification's target residue, shift every downstream coordinate (indels), or create/destroy a protease cut site — and the modifications have to follow correctly through all of it. This PR adds an executable specification for how that should behave: a small set of cases, each a tiny protein, run through the real load → variant-expand → digest pipeline and checked against a hand-authored expected answer.

The key discipline: expected answers are derived from the intended biology, never captured from current code — so the corpus catches bugs rather than enshrining them.

What's here (L0 foundation, test-only)

  • CorpusCase records in VariantCorpusTests.GetCases() — each case (base sequence, mods, variants, protease, caps, expected forms, verdict/reason) is an inline, compiler-checked record yielded to the tests via [TestCaseSource], matching the house style in Transcriptomics/TestDigestion.cs. Input and expected answer sit together; no external data files, no csproj copy-to-output plumbing.
  • ExpectedForms — the intended proteoforms, canonical least-modified-first order, in mzLib's real full-sequence notation (e.g. [Biological:Phosphorylation on T]). ExpectedCount rides alongside as an independent over/under-generation guard (mirrors RnaDigestionTestCase).
  • Two test families over the same oracle:
  • Five foundation cases (F00–F04): identity, single PTM, single substitution, sub×mod chemical-legality drop (3 forms not 4), and out-of-range pruning. All green against master, both families.

Why a corpus first

It turns a minefield into a measured map. As we add layers — indels + source-dependent encoding, post-translational processing, bottom-up digestion, multi-variant combinatorics, VCF depth/zygosity, read/write round-trips — the failing cases become the exact punch-list, and the natural boundaries for small, isolated fix-PRs.

Status

Draft / RFC. Deliberately small and test-only (no production code touched). This is the approach — looking for buy-in on the direction before building out the later layers. Review feedback folded in: cases now use ProteinDbWriter (round-trip) and live as typed records rather than an external table. L1 (indels + variant-borne mods) is where the first real red is expected.

🤖 Generated with Claude Code

The nut: an executable spec for how mzLib *should* read, expand, and digest
proteins that carry sequence variants and modifications together — one of the
most bug-prone corners in the library. Each case is a tiny protein described as
one row in a table; the test builds its XML, runs the real LoadProteinXML ->
variant-expansion -> top-down Digest pipeline, and compares the resulting
proteoforms to a hand-written expected answer. Expected answers are derived from
the intended biology, never captured from current code, so the corpus catches
bugs instead of enshrining them.

Details:
- cases.tsv = one row per case (base sequence, mods, variants, protease, caps,
  extensible opts tail); expected/<id>.txt = the intended forms, one per line in
  canonical "least-modified-first" order, in mzLib's real full-sequence notation.
- VariantCorpusTests asserts count + membership + run-twice determinism.
- L0 foundation (substitutions + PTMs, top-down, non-binding caps): identity,
  single mod, single substitution, sub-x-mod chemical-legality drop (3 forms not
  4), out-of-range pruning. All five green against master.
- Later layers extend the same table: indels + source-dependent encoding,
  processing, bottom-up digestion, multi-variant/order, VCF depth, round-trip.

Test-only; no production code touched.

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

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.46%. Comparing base (77fb7db) to head (a239e2d).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1084   +/-   ##
=======================================
  Coverage   91.46%   91.46%           
=======================================
  Files         427      427           
  Lines       52777    52777           
  Branches     6325     6325           
=======================================
  Hits        48273    48273           
  Misses       3268     3268           
  Partials     1236     1236           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@trishorts trishorts marked this pull request as ready for review July 1, 2026 12:31
@trishorts trishorts added Maintenance The user isn't impacted by it, it's purely behind the scenes ready for review labels Jul 1, 2026
string tmp = Path.Combine(Path.GetTempPath(), $"corpus_{row.Id}_{Guid.NewGuid():N}.xml");
try
{
File.WriteAllText(tmp, xml);

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.

Should we use Protein DB Writer here?

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.

Why store these in this manner instead of an xml database?

Adds Corpus_Node_RoundTrip: serializes the consensus protein through the
real ProteinDbWriter and re-reads it before digestion, so the corpus now
exercises encode+decode (invariant 6 / smith-chem-wisc#1083) as well as read+digest.

The writer never sits upstream of the ground truth -- the input XML is still
the hand-authored, bible-grounded one, and the round-trip reuses the same
expected/<id>.txt oracle. A writer that drops a variant or a variant-adjacent
mod fails here while Corpus_Node stays green. Green across L0 (F00-F04);
becomes the tripwire for variant-borne-mod and indel serialization at L1.

Addresses review feedback (@nbollis): use ProteinDbWriter in the harness.

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

Copy link
Copy Markdown
Contributor Author

Thanks @nbollis — both good calls. Addressed in ed7e509.

On ProteinDbWriter: added Corpus_Node_RoundTrip, which serializes the consensus protein through the real writer and re-reads it before digesting — a true encode/decode round-trip (invariant 6 / #1083). It reuses the same bible-derived oracle, so the writer is tested without ever sitting upstream of the ground truth (the input XML is still hand-authored, not writer-generated — otherwise a symmetric write/read bug could pass silently). Green across L0; becomes the tripwire for variant-borne-mod and indel serialization at L1, where I expect the first real red.

On storing the corpus as cases.tsv + expected/*.txt rather than XML databases:

  • cases.tsv is a spec table, not a database. Each row pairs an input with its expected_count, verdict, and reason (tied to a domain-model installment) — the documentation of why the answer is what it is. A reviewer scans the whole possibility tree and its verdicts on one screen; N <entry> elements can't carry that rationale and aren't scannable. The real mzLib XML is still exercised — each row expands into canonical UniProt XML through the real reader, and now the real writer.
  • expected/<id>.txt holds one FullSequence per line because the comparison unit is the proteoform — the direct output of digestion. Storing expected forms as XML would force a decode step to re-derive proteoforms before comparing, pulling a reader/writer into the oracle and reintroducing exactly the "expected values come from the domain model, never from our code" contamination the corpus forbids.
  • One input XML per case, not one combined database. Variant expansion is governed by parameters on the LoadProteinXML call (maxHeterozygousVariants, minAlleleDepth) that apply to the whole file, not per-entry — so later throttle/depth-policy layers couldn't give case A "capped at 2" and case B "uncapped" in the same load. Per-case files also keep each node hermetic and point failures at one minimal fixture.

Happy to fold any of this into the description if useful.

trishorts and others added 2 commits July 8, 2026 12:40
LoadRows parsed the spec table by magic index (c[7], int.Parse(c[8])...),
which obscures the columns and silently breaks on any reorder or added
column. Build the name->index map from the header once and construct Row
with named fields (S(base), I(expected_count)); fail loudly listing any
missing required column. cases.tsv is a deliberately growing table, so
header-binding is the robust reading.

Addresses review feedback (@nbollis): read the table into the typed object
in a way that documents the columns.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the external cases.tsv + expected/*.txt (and their csproj
copy-to-output directives) with a CorpusCase record yielded inline from
GetCases(), matching the house style in Transcriptomics/TestDigestion.cs.
Input spec and expected forms now sit together on one compiler-checked,
IDE-navigable record; no runtime file I/O, no file-not-found fragility.

ExpectedCount is kept alongside ExpectedForms as an independent
over/under-generation guard (asserted equal), mirroring RnaDigestionTestCase
keeping both DigestionProductCount and Sequences. Verdict/Reason ride along
as fields, preserving the spec documentation the table used to carry.
README updated to describe the record schema. Both families green (10/10).

Addresses review feedback (@nbollis): encode the cases into a class.

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

Copy link
Copy Markdown
Contributor Author

Good call — converted the corpus to inline CorpusCase records in 5a764bcb, following Transcriptomics/TestDigestion.cs.

  • cases.tsv + expected/*.txt (and their csproj copy-to-output directives) are gone; each case is now a compiler-checked record yielded from GetCases(), with the input spec and ExpectedForms sitting together. No runtime file I/O, no file-not-found fragility, IDE-navigable.
  • Kept ExpectedCount alongside ExpectedForms as an independent over/under-generation guard (asserted equal), mirroring how RnaDigestionTestCase keeps both DigestionProductCount and Sequences.
  • Verdict/Reason ride along as record fields, so the "why, tied to a bible installment" documentation the table carried isn't lost.

Both families green (10/10). Thanks for the two nudges — the writer round-trip and the typed cases both made this materially better. Description updated to match.

@trishorts trishorts merged commit 693b20e into smith-chem-wisc:master Jul 10, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Maintenance The user isn't impacted by it, it's purely behind the scenes ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants