Solve the bug of column name is accession.

src/mavedb/lib/csv/columns.py

@@ -59,6 +59,15 @@ def _value_or_na(value: Any, na_rep: str = NA_VALUE) -> str:
     return str(value)
 
 
+_CORE_COLUMN_NAMES = frozenset(namespace_spec(CORE_NAMESPACE).columns({}))  # type: ignore[union-attr]
+"""Core's own column names, so an investigator column of the same name can still be told apart.
+
+In practice only ``accession`` can collide: upload strips the HGVS columns out of the investigator's own
+score and count data (``create_variants_data`` in ``lib/score_sets.py``) before it is ever stored, so
+they never reach this module as a dataset column.
+"""
+
+
 def _format_column_key(namespace: str, column_key: str, namespaced: bool = False) -> str:
     """Shared key-formatting logic used by both header assembly and row assembly."""
     # Always namespaced regardless of the caller's preference: the release or calibration URN is what
@@ -69,7 +78,9 @@ def _format_column_key(namespace: str, column_key: str, namespaced: bool = False
     if namespace == CORE_NAMESPACE:  # core is never namespaced
         return column_key
 
-    if namespaced:
+    # Forced even for un-namespaced output: a dataset column sharing a core column's name would otherwise
+    # collide with it once the prefix is stripped. The core column itself keeps its bare name.
+    if namespaced or column_key in _CORE_COLUMN_NAMES:
         spec = namespace_spec(namespace)
         prefix = spec.emit_under if spec is not None and spec.emit_under is not None else namespace
         return f"{prefix}.{column_key}"
@@ -109,6 +120,10 @@ def plan_csv_columns(dataset_columns: dict, namespaces: list[str]) -> CsvColumnP
 def assemble_csv_headers(namespaced_columns: dict[str, list[str]], namespaced: bool = False) -> list[str]:
     """Build the flat column-header list from the namespace dict.
 
+    A dataset column named the same as a core column (only ``accession`` can happen in practice) keeps its
+    namespace prefix even in un-namespaced output, so it reads as e.g. ``scores.accession`` rather than
+    colliding with the bare ``accession`` core column.
+
     Raises:
         ValueError: if two namespaces resolve to the same header. Un-namespaced output strips the prefix
             that would otherwise keep them apart, so requesting two namespaces that share a column name

src/mavedb/lib/csv/specs.py

@@ -7,7 +7,12 @@
 
 from mavedb.lib.csv.namespaces import CALIBRATION_NS_PATTERN, CLINVAR_NS_PATTERN, CsvNamespace
 from mavedb.lib.mave.constants import REQUIRED_SCORE_COLUMN
-from mavedb.lib.validation.constants.general import hgvs_nt_column, hgvs_pro_column, hgvs_splice_column
+from mavedb.lib.validation.constants.general import (
+    accession_column,
+    hgvs_nt_column,
+    hgvs_pro_column,
+    hgvs_splice_column,
+)
 from mavedb.models.variant import Variant
 
 
@@ -141,7 +146,7 @@ def _optional(getter: Callable) -> Callable:
     CORE_NAMESPACE: CsvNamespaceSpec(
         source=RowSource.VARIANT,
         resolvers={
-            "accession": attrgetter("urn"),
+            accession_column: attrgetter("urn"),
             hgvs_nt_column: attrgetter(hgvs_nt_column),
             hgvs_splice_column: attrgetter(hgvs_splice_column),
             hgvs_pro_column: attrgetter(hgvs_pro_column),

src/mavedb/lib/csv/variant.py

@@ -38,6 +38,7 @@
     calibration_namespace_for_urn,
     clinvar_namespace_sort_key,
 )
+from mavedb.lib.exceptions import NonexistentIdentifierError
 from mavedb.lib.mave.utils import NA_VALUE
 from mavedb.lib.urns import score_set_urn_sort_key, variant_urn_sort_key
 from mavedb.lib.permissions.score_calibration import ScoreCalibrationViewer
@@ -255,14 +256,14 @@ def available_variant_csv_namespaces(
     equivalent measurements, and one ``clinvar.YYYY_MM`` per release covering them.
 
     Raises:
-        ValueError: if no variant with *variant_urn* exists.
+        NonexistentIdentifierError: if no variant with *variant_urn* exists.
     """
     measurements = _equivalent_measurements(db, variant_urn, may_read_score_set=may_read_score_set, as_of=as_of)
 
     if measurements is None:
         variant = db.scalars(select(Variant).where(Variant.urn == variant_urn).limit(1)).first()
         if variant is None:
-            raise ValueError(f"variant with URN '{variant_urn}' not found")
+            raise NonexistentIdentifierError(f"variant with URN '{variant_urn}' not found")
 
         # No mapping on this variant, but its score set may still be mapped, in which case the
         # mapping-derived columns are owed with NA rather than omitted.
@@ -306,7 +307,7 @@ def get_variant_csv(
             instant. Defaults to currently-live rows.
 
     Raises:
-        ValueError: if no variant with *variant_urn* exists.
+        NonexistentIdentifierError: if no variant with *variant_urn* exists.
     """
     measurements = _equivalent_measurements(db, variant_urn, may_read_score_set=may_read_score_set, as_of=as_of)
 
@@ -382,7 +383,7 @@ def _unmapped_variant_csv(
     ).one_or_none()
 
     if variant is None:
-        raise ValueError(f"variant with URN '{variant_urn}' not found")
+        raise NonexistentIdentifierError(f"variant with URN '{variant_urn}' not found")
 
     plan = plan_csv_columns(
         dataset_columns={},

src/mavedb/lib/validation/constants/general.py

@@ -36,6 +36,7 @@
 hgvs_splice_column = "hgvs_splice"
 hgvs_pro_column = "hgvs_pro"
 guide_sequence_column = "guide_sequence"
+accession_column = "accession"
 hgvs_columns = sorted([hgvs_nt_column, hgvs_pro_column, hgvs_splice_column])
 meta_data = "meta_data"
 score_columns = "score_columns"

src/mavedb/lib/validation/dataframe/dataframe.py

@@ -5,6 +5,7 @@
 
 from mavedb.lib.exceptions import MixedTargetError
 from mavedb.lib.validation.constants.general import (
+    accession_column,
     guide_sequence_column,
     hgvs_nt_column,
     hgvs_pro_column,
@@ -341,6 +342,9 @@ def validate_column_names(df: pd.DataFrame, kind: str, is_base_editor: bool) ->
     if len(columns) != len(set(columns)):
         raise ValidationError("duplicate column names are not allowed (this check is case insensitive)")
 
+    if accession_column in columns:
+        raise ValidationError(f"'{accession_column}' is a reserved column name (this check is case insensitive)")
+
     if is_base_editor:
         msg = "dataframes for base editor data must also define the '{0}' column"
         if guide_sequence_column not in columns:

tests/lib/csv/test_columns.py

@@ -101,6 +101,18 @@ def test_an_absent_column_does_not_disturb_its_neighbours(self):
             "count2": "5",
         }
 
+    def test_investigator_column_named_accession_is_namespaced_even_when_unnamespaced(self):
+        """A score column named `accession` must not collide with core's own `accession` column.
+
+        Headers (``assemble_csv_headers``) and row keys (here) share the same ``_format_column_key``
+        decision, so asserting it here is enough to cover both.
+        """
+        variant = MockVariant(urn="urn:mavedb:00000001-a-1#1", data={"score_data": {"accession": "sample-42"}})
+
+        row = variant_to_csv_row(variant, {"core": ["accession"], "scores": ["accession"]}, namespaced=False)
+
+        assert row == {"accession": "urn:mavedb:00000001-a-1#1", "scores.accession": "sample-42"}
+
 
 # ---------------------------------------------------------------------------
 # TestVariantToCsvRowUnrecognizedKey
@@ -416,6 +428,24 @@ def test_plan_csv_columns_reference_hgvs_namespace_populates_columns():
             False,
             ["accession", "clinvar.2024_01.clinical_significance"],
         ),
+        # An investigator column named `accession` collides with the core column of the same name, so it
+        # is forced under its namespace even though un-namespaced output would otherwise strip the prefix.
+        (
+            {"core": ["accession", "hgvs_nt"], "scores": ["score", "accession"]},
+            False,
+            ["accession", "hgvs_nt", "score", "scores.accession"],
+        ),
+        (
+            {"core": ["accession"], "counts": ["count1", "accession"]},
+            False,
+            ["accession", "count1", "counts.accession"],
+        ),
+        # `scores_custom` emits under `scores`, same as its non-colliding columns would.
+        (
+            {"core": ["accession"], "scores_custom": ["accession"]},
+            False,
+            ["accession", "scores.accession"],
+        ),
         # Mixed: respects insertion order
         (
             {

tests/lib/csv/test_variant.py

@@ -22,6 +22,7 @@
     available_variant_csv_namespaces,
     get_variant_csv,
 )
+from mavedb.lib.exceptions import NonexistentIdentifierError
 from mavedb.lib.mondo import get_generic_disease_term
 from mavedb.lib.permissions.principal import Principal
 from mavedb.lib.permissions.score_calibration import ScoreCalibrationViewer
@@ -315,6 +316,18 @@ def test_unknown_urn_raises(self, session, setup_lib_db_with_mapped_variant):
         with pytest.raises(ValueError, match="not found"):
             get_variant_csv(session, "urn:mavedb:00000001-a-1#999")
 
+    def test_unknown_urn_raises_nonexistent_identifier_error(self, session, setup_lib_db_with_mapped_variant):
+        """Raised as ``NonexistentIdentifierError`` specifically, which the app maps to a 404 response
+        rather than letting a deleted-mid-request variant fall through to a 500."""
+        with pytest.raises(NonexistentIdentifierError, match="not found"):
+            get_variant_csv(session, "urn:mavedb:00000001-a-1#999")
+
+    def test_available_namespaces_unknown_urn_raises_nonexistent_identifier_error(
+        self, session, setup_lib_db_with_mapped_variant
+    ):
+        with pytest.raises(NonexistentIdentifierError, match="not found"):
+            available_variant_csv_namespaces(session, "urn:mavedb:00000001-a-1#999")
+
     def test_no_calibration_yields_no_calibration_columns(self, session, setup_lib_db_with_mapped_variant):
         variant = setup_lib_db_with_mapped_variant.variant
 

tests/routers/test_score_set.py

@@ -24,6 +24,9 @@
 from mavedb.lib.annotation.exceptions import MappingDataDoesntExistException
 from mavedb.lib.exceptions import NonexistentOrcidUserError
 from mavedb.lib.score_distribution import summarize_scores
+from mavedb.lib.score_sets import columns_for_dataset, create_variants, create_variants_data, csv_data_to_df
+from mavedb.lib.validation.dataframe.dataframe import validate_and_standardize_dataframe_pair
+from mavedb.lib.validation.exceptions import ValidationError
 from mavedb.lib.validation.urn_re import MAVEDB_EXPERIMENT_URN_RE, MAVEDB_SCORE_SET_URN_RE, MAVEDB_TMP_URN_RE
 from mavedb.models.annotation_event import AnnotationEvent
 from mavedb.models.clinvar_allele_link import ClinvarAlleleLink
@@ -44,6 +47,7 @@
 from mavedb.routers.score_sets import _annotation_stream_record
 from mavedb.view_models.orcid import OrcidUser
 from mavedb.view_models.score_set import ScoreSet, ScoreSetCreate
+from mavedb.view_models.score_set_dataset_columns import DatasetColumnsCreate
 from tests.helpers.constants import (
     EXTRA_LICENSE,
     EXTRA_USER,
@@ -1753,6 +1757,42 @@ def test_upload_score_set_variant_data_returns_500_and_resets_processing_state_w
     assert pipelines == []
 
 
+def test_upload_score_set_variant_data_with_accession_column_is_accepted_but_fails_validation(
+    session, data_provider, client, setup_router_db, mock_s3_client, tmp_path
+):
+    """The router only parses the upload and enqueues a job (see `upload_score_set_variant_data`); the
+    real column-name validation runs later, in the worker job that creates variants. This checks both
+    halves of the fix: the HTTP boundary accepts a column named `accession` rather than 500ing, and the
+    validation the worker would run on this exact data rejects it with a typed, user-facing error instead
+    of letting it through or crashing.
+    """
+    experiment = create_experiment(client)
+    score_set = create_seq_score_set(client, experiment["urn"])
+
+    scores_csv_path = tmp_path / "scores_with_accession_column.csv"
+    scores_csv_path.write_text("hgvs_nt,hgvs_pro,score,accession\nc.1A>T,p.Thr1Ser,0.3,sample-1\n")
+
+    with (
+        open(scores_csv_path, "rb") as scores_file,
+        patch.object(arq.ArqRedis, "enqueue_job", return_value=None) as worker_queue,
+        patch.object(mock_s3_client, "upload_fileobj", return_value=None),
+    ):
+        response = client.post(
+            f"/api/v1/score-sets/{score_set['urn']}/variants/data",
+            files={"scores_file": (scores_csv_path.name, scores_file, "text/csv")},
+        )
+        worker_queue.assert_called_once()
+
+    assert response.status_code == 200
+
+    item = session.scalars(select(ScoreSetDbModel).where(ScoreSetDbModel.urn == score_set["urn"])).one()
+    with open(scores_csv_path, "rb") as scores_file:
+        score_df = csv_data_to_df(scores_file)
+
+    with pytest.raises(ValidationError, match="'accession' is a reserved column name"):
+        validate_and_standardize_dataframe_pair(score_df, None, None, None, item.target_genes, data_provider)
+
+
 def test_upload_score_set_variant_data_deletes_s3_files_and_pipeline_when_enqueue_job_fails(
     session, client, setup_router_db, data_files, mock_s3_client
 ):
@@ -4089,6 +4129,73 @@ def test_download_counts_file(session, data_provider, client, setup_router_db, d
     assert "hgvs_splice" not in columns
 
 
+def test_download_scores_and_counts_with_accession_dataset_column_returns_200(
+    session, data_provider, client, setup_router_db, tmp_path
+):
+    """A score set whose score/count data already has a column literally named `accession` (data ingested
+    before uploads rejected that name) must still download rather than 500ing on the header collision with
+    the core `accession` column; the investigator's column is namespaced instead (`scores.accession` /
+    `counts.accession`). New uploads can no longer acquire such a column (see the upload-side test above),
+    so this models a pre-existing score set directly rather than going through the upload endpoint.
+    """
+    experiment = create_experiment(client)
+    score_set = create_seq_score_set(client, experiment["urn"])
+
+    scores_csv_path = tmp_path / "scores_with_accession_column.csv"
+    scores_csv_path.write_text(
+        "hgvs_nt,hgvs_pro,score,accession\nc.1A>T,p.Thr1Ser,0.3,sample-1\nc.2C>T,p.Thr1Met,1.0,sample-2\nc.6T>A,p.Phe2Leu,-1.65,sample-3\n"
+    )
+    # A counts dataframe's non-hgvs columns must all be numeric (`validate_data_column`, force_numeric=True
+    # for kind="counts"), so unlike the scores file above, `accession` here has to be numeric too.
+    counts_csv_path = tmp_path / "counts_with_accession_column.csv"
+    counts_csv_path.write_text(
+        "hgvs_nt,hgvs_pro,count1,accession\nc.1A>T,p.Thr1Ser,10,1\nc.2C>T,p.Thr1Met,8,2\nc.6T>A,p.Phe2Leu,90,3\n"
+    )
+    with open(scores_csv_path, "rb") as scores_file:
+        scores_df = csv_data_to_df(scores_file)
+    with open(counts_csv_path, "rb") as counts_file:
+        counts_df = csv_data_to_df(counts_file)
+
+    item = session.scalars(select(ScoreSetDbModel).where(ScoreSetDbModel.urn == score_set["urn"])).one()
+
+    # Bypasses only the reserved-`accession`-column check, to model data that predates it; everything else
+    # runs through the real standardization pipeline.
+    with patch("mavedb.lib.validation.dataframe.dataframe.validate_column_names", return_value=None):
+        scores, counts, score_columns_metadata, count_columns_metadata = validate_and_standardize_dataframe_pair(
+            scores_df, counts_df, None, None, item.target_genes, data_provider
+        )
+
+    variants = create_variants_data(scores, counts, None)
+    num_variants = create_variants(session, item, variants)
+    assert num_variants == len(scores)
+
+    item.processing_state = ProcessingState.success
+    item.dataset_columns = DatasetColumnsCreate(
+        score_columns=columns_for_dataset(scores),
+        count_columns=columns_for_dataset(counts),
+        score_columns_metadata=score_columns_metadata if score_columns_metadata is not None else {},
+        count_columns_metadata=count_columns_metadata if count_columns_metadata is not None else {},
+    ).model_dump()
+    session.add(item)
+    session.commit()
+
+    with patch.object(arq.ArqRedis, "enqueue_job", return_value=None) as worker_queue:
+        published_score_set = publish_score_set(client, score_set["urn"])
+        worker_queue.assert_called_once()
+
+    scores_response = client.get(f"/api/v1/score-sets/{published_score_set['urn']}/scores")
+    assert scores_response.status_code == 200
+    scores_header = scores_response.text.splitlines()[0].split(",")
+    assert "accession" in scores_header
+    assert "scores.accession" in scores_header
+
+    counts_response = client.get(f"/api/v1/score-sets/{published_score_set['urn']}/counts")
+    assert counts_response.status_code == 200
+    counts_header = counts_response.text.splitlines()[0].split(",")
+    assert "accession" in counts_header
+    assert "counts.accession" in counts_header
+
+
 @pytest.mark.parametrize("path", ["scores", "counts", "variants/data"])
 def test_csv_routes_raise_the_statement_timeout(session, data_provider, client, setup_router_db, data_files, path):
     experiment = create_experiment(client)

tests/validation/dataframe/test_dataframe.py

@@ -6,6 +6,7 @@
 import pytest
 
 from mavedb.lib.validation.constants.general import (
+    accession_column,
     guide_sequence_column,
     hgvs_nt_column,
     hgvs_pro_column,
@@ -387,6 +388,32 @@ def test_duplicate_data_column_names_counts(self):
                 is_base_editor=False,
             )
 
+    def test_column_named_accession_is_rejected_scores(self):
+        with self.assertRaises(ValidationError):
+            validate_column_names(
+                self.dataframe.rename(columns={"extra": accession_column}), kind="scores", is_base_editor=False
+            )
+
+    def test_column_named_accession_is_rejected_counts(self):
+        with self.assertRaises(ValidationError):
+            validate_column_names(
+                self.dataframe.drop([required_score_column], axis=1).rename(columns={"extra": accession_column}),
+                kind="counts",
+                is_base_editor=False,
+            )
+
+    def test_column_named_accession_is_rejected_case_insensitively(self):
+        with self.assertRaises(ValidationError):
+            validate_column_names(
+                self.dataframe.rename(columns={"extra": accession_column.upper()}),
+                kind="scores",
+                is_base_editor=False,
+            )
+
+    def test_hgvs_columns_are_still_accepted_alongside_accession_rejection(self):
+        """The accession rejection must not collide with the HGVS columns, which are always present."""
+        validate_column_names(self.dataframe, kind="scores", is_base_editor=False)
+
     # Written without @pytest.mark.parametrize. See: https://pytest.org/en/7.4.x/how-to/unittest.html#pytest-features-in-unittest-testcase-subclasses
     def test_invalid_column_names_scores(self):
         invalid_values = [None, np.nan, "", " "]

tests/view_models/test_allele_measurement.py

@@ -1,3 +1,9 @@
+# ruff: noqa: E402
+
+import pytest
+
+fastapi = pytest.importorskip("fastapi")
+
 from mavedb.view_models.allele_measurement import AlleleMeasurement
 from tests.helpers.constants import TEST_SAVED_FUNCTIONAL_RANGE_ABNORMAL