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

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).

The HGVS parser preserves the input reference allele (e.g. the C in
NM_006087.3:c.900C>A), but the translator never checked it against the
reference sequence, so an incorrect reference allele silently produced a
plausible-but-wrong VRS Allele (ga4gh#364).

extract_allele_values now validates the stated reference allele against
the data proxy (default require_validation=True), mirroring the existing
gnomAD translator behavior. require_validation=False keeps the legacy
behavior (mismatch only logged). Adds hermetic regression tests and the
VCR cassette interactions the new lookups require.
@developer-rpai
developer-rpai requested review from a team as code owners September 24, 2026 05:15

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