Harden VA-Spec annotation and CSV exports against shape-dependent failure

src/mavedb/lib/annotation/conformance.py

@@ -0,0 +1,58 @@
+"""Structural conformance checks for emitted VA-Spec annotations.
+
+An annotation that constructs successfully is not necessarily one a consumer can read back. Two
+defects of exactly that shape have reached production: a required ``Extension.value`` combined with a
+null baseline score stripped by ``model_dump(exclude_none=True)`` (fixed in 5c155f4d), and a
+reference-identical variant whose VRS state is a ``ReferenceLengthExpression`` (fixed in 84125081).
+Neither was caught by a test that only asserted the object had been built.
+
+This module is declared here so it can be run from both the test suite and from script based conformance
+checks.
+"""
+
+import json
+from typing import TypeVar
+
+from ga4gh.va_spec.acmg_2015 import VariantPathogenicityStatement
+from ga4gh.va_spec.base.core import ExperimentalVariantFunctionalImpactStudyResult, Statement
+
+Annotation = TypeVar(
+    "Annotation", ExperimentalVariantFunctionalImpactStudyResult, Statement, VariantPathogenicityStatement
+)
+
+
+class AnnotationRoundTripError(Exception):
+    """An emitted annotation did not survive serialization and re-validation."""
+
+
+def round_trip_annotation(annotation: Annotation) -> Annotation:
+    """Serialize an annotation the way the API emits it, then read it back.
+
+    Mirrors the emission path exactly — ``model_dump(exclude_none=True)`` then ``json.dumps(default=str)``
+    — because the failures worth catching are the ones those two steps introduce.
+
+    The assertion is that emission is a fixed point: the re-validated object dumps to the same JSON it
+    was parsed from. Object equality would be the wrong test. VA-Spec declares container fields in terms
+    of base classes — ``Statement.hasEvidenceLines`` is a ``list[EvidenceLine]`` — so a
+    ``VariantPathogenicityEvidenceLine`` re-validates as a plain ``EvidenceLine``. That narrowing loses
+    no data and produces byte-identical JSON, so it is not a defect this check should report.
+
+    Returns the re-validated object.
+
+    Raises:
+        AnnotationRoundTripError: The emitted JSON did not re-validate, or re-validated to something
+            that serializes differently than what was emitted.
+    """
+    model = type(annotation)
+    emitted = json.dumps(annotation.model_dump(exclude_none=True), default=str)
+
+    try:
+        reparsed = model.model_validate(json.loads(emitted))
+    except Exception as err:
+        raise AnnotationRoundTripError(f"{model.__name__} did not re-validate after emission: {err}") from err
+
+    re_emitted = json.dumps(reparsed.model_dump(exclude_none=True), default=str)
+    if re_emitted != emitted:
+        raise AnnotationRoundTripError(f"{model.__name__} did not survive a round trip unchanged.")
+
+    return reparsed

src/mavedb/lib/annotation/exceptions.py

@@ -1,2 +1,10 @@
 class MappingDataDoesntExistException(ValueError):
     pass
+
+
+#: Exceptions meaning a variant has nothing to annotate, as opposed to something going wrong while
+#: annotating it. Every caller that reports failures has to tell the two apart: the streaming endpoints
+#: emit an expected absence as a null annotation rather than an error record, and the corpus sweep counts
+#: it as not-applicable rather than a defect. Adding an entry here changes both, and they must agree —
+#: a sweep that treated an expected absence as a failure would bury real defects in noise.
+EXPECTED_ABSENCE_EXCEPTIONS = (MappingDataDoesntExistException,)

src/mavedb/lib/csv/specs.py

@@ -109,13 +109,31 @@ def resolver(self, column_key: str) -> Optional[Callable]:
         return _optional(lambda data: data.get(column_key))
 
 
+def _safe_hgvs_from_post_mapped(mapping: MappedVariant) -> Optional[str]:
+    """``get_hgvs_from_post_mapped`` with its raises absorbed, since a cell cannot afford to raise.
+
+    ``hgvs_from_vrs_allele`` raises on two payload shapes: a VRS 1.x object, which it refuses
+    deliberately, and an allele with no ``expressions`` key. Raised from inside a cell resolver, either
+    would abort the export of every other row in the file. Returning None keeps the refusal — no HGVS is
+    emitted for those payloads — without letting one variant's shape cost the caller the whole download.
+
+    Intentionally scoped to the CSV export layer, where a single problematic variant should not abort the entire export.
+    """
+    if not mapping.post_mapped:
+        return None
+    try:
+        return get_hgvs_from_post_mapped(mapping.post_mapped)
+    except (KeyError, ValueError):
+        return None
+
+
 def _post_mapped_hgvs_g(mapping: Optional[MappedVariant]) -> Optional[str]:
     """The genomic HGVS expression, falling back to one parsed out of the post-mapped VRS object."""
     if mapping is None:
         return None
     if mapping.hgvs_g:
         return str(mapping.hgvs_g)
-    fallback = get_hgvs_from_post_mapped(mapping.post_mapped) if mapping.post_mapped else None
+    fallback = _safe_hgvs_from_post_mapped(mapping)
     return fallback if fallback is not None and is_hgvs_g(fallback) else None
 
 
@@ -125,7 +143,7 @@ def _post_mapped_hgvs_p(mapping: Optional[MappedVariant]) -> Optional[str]:
         return None
     if mapping.hgvs_p:
         return str(mapping.hgvs_p)
-    fallback = get_hgvs_from_post_mapped(mapping.post_mapped) if mapping.post_mapped else None
+    fallback = _safe_hgvs_from_post_mapped(mapping)
     return fallback if fallback is not None and is_hgvs_p(fallback) else None
 
 

src/mavedb/lib/middleware/__init__.py

@@ -0,0 +1,7 @@
+"""ASGI middleware for the MaveDB application."""
+
+from mavedb.lib.middleware.errors import CatchAllErrorMiddleware
+
+__all__ = [
+    "CatchAllErrorMiddleware",
+]

src/mavedb/lib/middleware/errors.py

@@ -0,0 +1,75 @@
+"""Middleware that converts an uncaught exception into a response the caller can read."""
+
+import logging
+import time
+
+from starlette.requests import Request
+from starlette.responses import JSONResponse
+from starlette.types import ASGIApp, Message, Receive, Scope, Send
+
+from mavedb.lib.logging.canonical import log_request
+from mavedb.lib.logging.context import (
+    correlation_id_for_context,
+    format_raised_exception_info_as_dict,
+    logging_context,
+    save_to_logging_context,
+)
+from mavedb.lib.slack import send_slack_error
+
+logger = logging.getLogger(__name__)
+
+
+class CatchAllErrorMiddleware:
+    """Turn an uncaught exception into a 500 that the browser is allowed to read.
+
+    Starlette dispatches ``@app.exception_handler(Exception)`` from ``ServerErrorMiddleware``, which sits
+    outside the user middleware stack. Its response therefore never passes through ``CORSMiddleware`` and
+    carries no ``Access-Control-Allow-Origin``, so a browser rejects it before the client library sees the
+    body and the caller is left with an opaque network error. Installing this middleware *inside* the CORS
+    layer puts the 500 back under CORS, and inside the context middleware so the correlation id that
+    identifies the failure in the logs can be returned to the caller.
+
+    Implemented as pure ASGI rather than ``BaseHTTPMiddleware``: the latter interposes on the response body
+    and has a history of breaking ``StreamingResponse``, which the NDJSON export endpoints rely on. Only
+    exceptions raised before the response starts are converted — once bytes are on the wire the status is
+    already committed, so the exception is re-raised and each stream is responsible for its own
+    mid-flight error reporting.
+    """
+
+    def __init__(self, app: ASGIApp) -> None:
+        self.app = app
+
+    async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
+        if scope["type"] != "http":
+            await self.app(scope, receive, send)
+            return
+
+        response_started = False
+
+        async def send_wrapper(message: Message) -> None:
+            nonlocal response_started
+            if message["type"] == "http.response.start":
+                response_started = True
+            await send(message)
+
+        try:
+            await self.app(scope, receive, send_wrapper)
+        except Exception as err:
+            # The status line is already committed; nothing can be salvaged into a 500 here.
+            if response_started:
+                raise
+
+            save_to_logging_context(format_raised_exception_info_as_dict(err))
+            request = Request(scope, receive)
+            response = JSONResponse(
+                status_code=500,
+                content={"detail": "Internal server error", "correlation_id": correlation_id_for_context()},
+            )
+
+            try:
+                logger.error(msg="Uncaught exception.", extra=logging_context(), exc_info=err)
+                send_slack_error(err=err, request=request)
+            finally:
+                log_request(request, response, time.time_ns())
+
+            await response(scope, receive, send)