From ccf3eeff8c931a1d602cef706eecadc37a600e23 Mon Sep 17 00:00:00 2001
From: Benjamin Capodanno <bencap@outlook.com>
Date: Fri, 28 Aug 2026 10:27:26 -0700
Subject: [PATCH] fix: return 4XX for malformed input to hgvs/validate

The parse step in hgvs_validate ran outside the try/except, so an
HGVSParseError on a malformed variant string escaped to the catch-all
500 handler (and fired a Slack alert). A missing "variant" body field
raised KeyError with the same outcome.

Parse inside the error handler and catch the broad HGVSError so any
caller-supplied HGVS failure (unparseable string, inconsistent variant,
unknown accession) becomes a 400. Replace the free-form dict body with a
typed HgvsValidationRequest model, which makes a missing/mistyped field
a 422 and self-documents the request schema.
---
 src/mavedb/routers/hgvs.py     | 22 ++++++++++++----------
 src/mavedb/view_models/hgvs.py |  5 +++++
 tests/routers/test_hgvs.py     | 16 ++++++++++++++++
 3 files changed, 33 insertions(+), 10 deletions(-)
 create mode 100644 src/mavedb/view_models/hgvs.py

diff --git a/src/mavedb/routers/hgvs.py b/src/mavedb/routers/hgvs.py
index c2352c81d..ac6277300 100644
--- a/src/mavedb/routers/hgvs.py
+++ b/src/mavedb/routers/hgvs.py
@@ -5,10 +5,11 @@
 from cdot.hgvs.dataproviders import RESTDataProvider
 from fastapi import APIRouter, Depends, HTTPException
 from hgvs import parser, validator
-from hgvs.exceptions import HGVSDataNotAvailableError, HGVSInvalidVariantError
+from hgvs.exceptions import HGVSDataNotAvailableError, HGVSError
 
 from mavedb.deps import hgvs_data_provider
-from mavedb.routers.shared import BASE_400_RESPONSE, PUBLIC_ERROR_RESPONSES, ROUTER_BASE_PREFIX
+from mavedb.routers.shared import PUBLIC_ERROR_RESPONSES, ROUTER_BASE_PREFIX, VALIDATION_ERROR_RESPONSES
+from mavedb.view_models.hgvs import HgvsValidationRequest
 
 TAG_NAME = "Transcripts"
 
@@ -44,22 +45,23 @@ def hgvs_fetch(accession: str, hdp: RESTDataProvider = Depends(hgvs_data_provide
     "/validate",
     status_code=200,
     response_model=bool,
-    responses={**BASE_400_RESPONSE},
+    responses={**VALIDATION_ERROR_RESPONSES},
     summary="Validate a provided variant",
 )
-def hgvs_validate(variant: dict[str, str], hdp: RESTDataProvider = Depends(hgvs_data_provider)) -> bool:
+def hgvs_validate(request: HgvsValidationRequest, hdp: RESTDataProvider = Depends(hgvs_data_provider)) -> bool:
     """
     Validate the provided HGVS variant string.
+
+    Parsing and validation failures both stem from caller-supplied input, so any ``HGVSError`` — a syntactic
+    parse failure, an inconsistent variant, an unknown accession — is surfaced as a 400 rather than escaping
+    to the catch-all 500 handler.
     """
     hp = parser.Parser()
-    variant_hgvs = hp.parse(variant["variant"])
-
     try:
-        valid = validator.Validator(hdp=hdp).validate(variant_hgvs, strict=False)
-    except HGVSInvalidVariantError as e:
+        variant_hgvs = hp.parse(request.variant)
+        return validator.Validator(hdp=hdp).validate(variant_hgvs, strict=False)
+    except HGVSError as e:
         raise HTTPException(400, str(e))
-    else:
-        return valid
 
 
 @router.get("/assemblies", status_code=200, response_model=list[str], summary="List stored assemblies")
diff --git a/src/mavedb/view_models/hgvs.py b/src/mavedb/view_models/hgvs.py
new file mode 100644
index 000000000..e295cc9df
--- /dev/null
+++ b/src/mavedb/view_models/hgvs.py
@@ -0,0 +1,5 @@
+from mavedb.view_models.base.base import BaseModel
+
+
+class HgvsValidationRequest(BaseModel):
+    variant: str
diff --git a/tests/routers/test_hgvs.py b/tests/routers/test_hgvs.py
index 6011953f2..b8a689dfb 100644
--- a/tests/routers/test_hgvs.py
+++ b/tests/routers/test_hgvs.py
@@ -24,6 +24,7 @@
 INVALID_TRANSCRIPT = "NX_99999.1"
 VALID_VARIANT = VALID_NT_ACCESSION + ":c.1G>A"
 INVALID_VARIANT = VALID_NT_ACCESSION + ":c.1delA"
+UNPARSEABLE_VARIANT = "not a valid hgvs string"
 HAS_PROTEIN_ACCESSION = "NM_000014.4"
 PROTEIN_ACCESSION = "NP_000005.2"
 
@@ -64,6 +65,21 @@ def test_hgvs_validate_invalid(client, setup_router_db):
         assert "does not agree" in response.json()["detail"]
 
 
+def test_hgvs_validate_unparseable(client, setup_router_db):
+    # A syntactically invalid HGVS string fails at the parse stage, before validation. This is a caller
+    # error and must surface as a 400 rather than escaping to the catch-all 500 handler.
+    payload = {"variant": UNPARSEABLE_VARIANT}
+    response = client.post("/api/v1/hgvs/validate", json=payload)
+
+    assert response.status_code == 400
+
+
+def test_hgvs_validate_missing_field(client, setup_router_db):
+    response = client.post("/api/v1/hgvs/validate", json={})
+
+    assert response.status_code == 422
+
+
 def test_hgvs_list_assemblies(client, setup_router_db):
     response = client.get("/api/v1/hgvs/assemblies")
     assert response.status_code == 200