fix(variants): read UniProt-native deletions (empty <variation>)#1095
Open
trishorts wants to merge 3 commits into
Open
fix(variants): read UniProt-native deletions (empty <variation>)#1095trishorts wants to merge 3 commits into
trishorts wants to merge 3 commits into
Conversation
The ProteinXmlEntry reader admitted a sequence-variant feature only when <variation> was non-empty, so it silently dropped every UniProt-native deletion (encoded as <original>X</original><variation/>, which is how UniProt writes a deletion and is not ours to change). Admit any feature that encodes a real change -- not both <original> and <variation> empty -- so deletions become SequenceVariations alongside substitutions/insertions. Corpus (L1 deletions, bible-derived oracle, red-before-fix): add nodes D00-D08 and MD00-MD02 -- single/multi-base deletions, deletions before/on/ after a PTM residue (phospho re-anchors/drops/stays), out-of-range and straddle-end pruning, the VCF depth cutoff (minAlleleDepth) pass/fail, and pairs of separate deletions (capped at 2). Harness gains BEGIN/END, empty VAR (deletion), SRC=vcf (GT/AD/DP), and a per-case MinAlleleDepth. Fix a latent fixture error the drop had masked: in seqvartests.xml the YPSE deletion declared begin=960, but YPSE sits at 961-964 (base is NYPSE at 960-964), violating SeqVarXmlTest's invariant that OriginalSequence equals the residues at [begin,end]. Corrected begin 960->961. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GbKQ6fMLGsmiPAHPN3W6gz
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1095 +/- ##
=======================================
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:
|
…variants; isolate D06 end>len Corpus_Node_RoundTrip previously asserted only the digested proteoform set, which for consensus-only nodes equals the base sequence whether or not the writer preserved the SequenceVariation. Add AssertConsensusVariationsRoundTrip comparing the consensus SequenceVariations (position + original + variant) before write vs after re-read, so a writer that drops a read-but-unapplied variant now fails the round-trip (real teeth on D08, the depth-filtered node whose variant is retained on the consensus). Out-of-range nodes (D05/D06) are pruned from the consensus at load, so both sides are legitimately empty. Retune D06 to OP=E BEGIN=7 END=8 so end>len is the sole reason for pruning (the original now matches residues[begin,end] in range), cleanly isolating the PruneOutOfRangeSequenceVariants end-beyond-length branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
Manual unit-test reviewAll 14 unit test(s) added by this PR were reviewed individually by the author (not only by automated/AI-generated checks): 13 approved as written, 1 edited, 0 flagged. This satisfies the author's standing AI-use policy that AI-assisted tests receive human review before relying on them. Recorded 2026-07-11 06:54. |
…inal YPSE deletion SeqVarXmlTest only asserts that P40467's UniProt-native YPSE deletion (empty <variation/> at 961-964, admitted by the smith-chem-wisc#1095 reader fix) is READ with the right coordinates. Add SeqVar_NativeCTerminalDeletion_YPSE_AppliesAndShortensProtein to assert it is APPLIED correctly: taking the deletion exactly as read from the fixture and applying it in isolation yields a proteoform that ends ...YLPLN (length - 4), not ...YLPLNYPSE, with the applied variation carrying original "YPSE", empty variant, and post-collapse coordinates 961..960. A bug that read the native deletion yet failed to apply it would pass SeqVarXmlTest and fail here. Scoped to the deletions tranche; P40467's substitutions/insertions are left for their own tranches. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
What
Fixes the
ProteinXmlEntryreader so it reads UniProt-native deletions, which UniProt encodes as an empty<variation/>(e.g.<original>P</original><variation/>). The reader previously admitted asequence variantfeature only when<variation>was non-empty, so every UniProt-native deletion was silently dropped — noSequenceVariationwas ever created. This is not something we can push onto the encoding: it is how UniProt writes a deletion.The guard now admits any feature that encodes a real change (not both
<original>and<variation>empty), so deletions are read alongside substitutions and insertions.SequenceVariation, the writer, and the round-trip already support an emptyVariantSequence, so no other production change is needed.Corpus (L1 deletions — executable spec, oracle derived from the domain model, red-before-fix)
New nodes in
Test/DatabaseTests/VariantCorpus:minAlleleDepth), same deletion passing at cutoff 5 (2 forms) / filtered at 7 (1 form)D00–D04 fail on master (reproducing the reader gap) and go green with the fix. The harness gains
BEGIN/END, empty-VAR(deletion),SRC=vcf(GT/AD/DP) support, and a per-caseMinAlleleDepth.Buggy fixture the drop was masking
Admitting the deletions surfaced a latent error in
seqvartests.xml: theYPSEdeletion declaredbegin=960, butYPSEsits at 961–964 (the base isNYPSEat 960–964), violatingSeqVarXmlTest's own invariant thatOriginalSequenceequals the residues at[begin,end]. It never ran because the variant was being dropped. Correctedbegin960→961.Verification
VariantCorpus(incl.ProteinDbWriterround-trip + run-twice determinism)DatabaseTests+Transcriptomics🤖 Generated with Claude Code