From cd9446e46f0f9618456bea887533d8e5c08255e3 Mon Sep 17 00:00:00 2001
From: Daniel Weindl <daniel.weindl@uni-bonn.de>
Date: Tue, 28 Oct 2025 22:21:04 +0100
Subject: [PATCH] v2: validate mappings

https://petab.readthedocs.io/en/latest/v2/documentation_data_format.html#mapping-table:
> The petabEntityId may be the same as the modelEntityId, but it must not be used to alias an entity that already has a valid PEtab identifier. This restriction is to avoid unnecessary complexity in the PEtab problem files.
---
 petab/v2/core.py      | 16 ++++++++++++++++
 tests/v2/test_core.py | 34 ++++++++++++++++++++++++++++++++--
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/petab/v2/core.py b/petab/v2/core.py
index 4ced4b9c..5e8c59d0 100644
--- a/petab/v2/core.py
+++ b/petab/v2/core.py
@@ -846,6 +846,22 @@ class Mapping(BaseModel):
         populate_by_name=True, extra="allow", validate_assignment=True
     )
 
+    @model_validator(mode="after")
+    def _validate(self) -> Self:
+        if (
+            self.model_id
+            and self.model_id != self.petab_id
+            and is_valid_identifier(self.model_id)
+        ):
+            raise ValueError(
+                "Aliasing of entities that already have a valid identifier "
+                "is not allowed. Simplify your PEtab problem by removing the "
+                f"mapping entry for `{self.petab_id} -> {self.model_id}`, "
+                f"and replacing all occurrences of `{self.petab_id}` with "
+                f"`{self.model_id}`."
+            )
+        return self
+
 
 class MappingTable(BaseTable[Mapping]):
     """PEtab mapping table."""
diff --git a/tests/v2/test_core.py b/tests/v2/test_core.py
index 64e1ad41..060fbb32 100644
--- a/tests/v2/test_core.py
+++ b/tests/v2/test_core.py
@@ -524,12 +524,12 @@ def test_modify_problem():
         check_dtype=False,
     )
 
-    problem.add_mapping("new_petab_id", "some_model_entity_id")
+    problem.add_mapping("new_petab_id", "1some_model_entity_id")
 
     exp_mapping_df = pd.DataFrame(
         data={
             PETAB_ENTITY_ID: ["new_petab_id"],
-            MODEL_ENTITY_ID: ["some_model_entity_id"],
+            MODEL_ENTITY_ID: ["1some_model_entity_id"],
             NAME: [None],
         }
     ).set_index([PETAB_ENTITY_ID])
@@ -836,3 +836,33 @@ def test_get_output_parameters():
     assert petab_problem.get_output_parameters(
         observable=False, noise=True
     ) == ["p1", "p3", "p5"]
+
+
+def test_mapping_validation():
+    """Test that invalid mapping entries raise errors."""
+
+    # alias invalid model entity ID
+    Mapping(
+        petab_id="valid_id",
+        model_id=" 1_invalid",
+    )
+
+    with pytest.raises(ValidationError, match="Invalid ID"):
+        # invalid petab entity ID
+        Mapping(
+            petab_id="1_invalid",
+            model_id="valid_id",
+        )
+
+    with pytest.raises(ValidationError, match="Aliasing.*not allowed"):
+        # unnecessary aliasing is forbidden
+        Mapping(
+            petab_id="forbidden_alias_of_valid_id",
+            model_id="valid_id",
+        )
+
+    # missing model_id is valid (annotation-only entry)
+    Mapping(petab_id="valid_id", name="some name")
+
+    # identity mapping is valid
+    Mapping(petab_id="valid_id", model_id="valid_id", name="some name")