Skip to content

fix: validate reference allele stated in HGVS expressions - #655

Open
developer-rpai wants to merge 1 commit into
ga4gh:mainfrom
developer-rpai:fix-364-hgvs-ref-validation
Open

developer-rpai wants to merge 1 commit into
ga4gh:mainfrom
developer-rpai:fix-364-hgvs-ref-validation

Conversation

@developer-rpai

Copy link
Copy Markdown
Contributor

Summary

Closes #364.

HGVS-to-VRS translation silently accepted an incorrect reference allele: NM_006087.3:c.900C>A produced a plausible-looking VRS Allele even though the true reference base is G (per the ClinGen Allele Registry: "IncorrectReferenceAllele ... given=C, found=G", independently confirmed against NCBI RefSeq: with the CDS annotated at 373..1707, c.900 maps to n.1272, and NM_006087.3 n.1272 is G).

Root cause

The HGVS parser preserves the stated reference allele (sv.posedit.edit.ref), but HgvsTools.extract_allele_values() never inspected it, so a wrong reference flowed straight into the emitted Allele. The gnomAD translator path already validated reference alleles via DataProxy.validate_ref_seq(); the HGVS path did not.

Change

  • HgvsTools.extract_allele_values() accepts require_validation: bool = True. After any c.->n. mapping, when the HGVS expression states a reference allele (e.g. the C in c.900C>A, or deleted bases in g.44908822delC), it is validated against the reference sequence, raising DataProxyValidationError on mismatch; with require_validation=False the mismatch is only logged.
  • AlleleTranslator._from_hgvs() passes require_validation through, so the existing translate_from(..., require_validation=False) escape hatch now applies to HGVS input too.
  • Default behavior: refuse to emit an invalid Allele. Edits that state no reference allele (insertions, duplications, bare deletions) skip validation entirely.

Tests

  • New hermetic regression tests in tests/extras/test_allele_translator.py (no UTA/seqrepo network needed): the exact issue variant raises by default; a del-with-ref mismatch raises; correct-reference controls (substitution and deletion-with-ref) still translate; require_validation=False returns the allele with a logged warning; edits without a stated ref skip validation.
  • Negative control: with the src/ fix reverted, the two "raises" tests fail (pristine code emits the Allele); with the fix, all 5 pass.
  • Added the VCR cassette interactions required by the new reference lookups in test_from_hgvs and the test_hgvs substitution cases. Ground truth for the added interactions was cross-checked against the repo's own previously recorded cassettes and NCBI RefSeq.
  • Ruff check and format --check pass on all touched files.

Limitations

  • The UTA-backed VCR tests (test_from_hgvs, test_hgvs) could not be executed in this environment (no UTA or seqrepo access). Cassette completeness was verified by replaying the fixed code path against the recorded cassettes with the session-scoped cache shared, but CI is the authoritative check.
  • 3 pre-existing test_vrs_normalize.py failures (VCR cassette mismatches) occur identically with and without this change.

Related: #151 (maintainer discussion supports validating supplied reference alleles and raising rather than emitting invalid objects).

@developer-rpai
developer-rpai requested review from a team as code owners September 24, 2026 05:15
Rebased onto upstream main (resolves the test_allele_translator.py
conflict with the ga4gh#489 from-VRS regression test); change unchanged:

- HgvsTools.extract_allele_values gains require_validation: when the HGVS
  expression states a reference allele (e.g. the C in c.900C>A), it is
  validated against the reference sequence via DataProxy.validate_ref_seq
  (ga4gh#364); mismatch raises DataProxyValidationError unless
  require_validation=False
- AlleleTranslator._from_hgvs threads require_validation through
- Hermetic regression tests with a canned data proxy (no seqrepo/UTA)
@developer-rpai
developer-rpai force-pushed the fix-364-hgvs-ref-validation branch from 6624c3d to 1d9ca7e Compare October 7, 2026 07:19

This branch has not been deployed

No deployments
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.

hgvs to vrs is returning valid results when hgvs has IncorrectReferenceAllele

1 participant