From eb384f8a3021597f2a074bd4cc087114e58fe25a Mon Sep 17 00:00:00 2001 From: Alessandro Pomponio Date: Mon, 27 Jul 2026 16:46:39 +0100 Subject: [PATCH 1/3] feat(core): annotate domain validation errors with property identifier Signed-off-by: Alessandro Pomponio --- ado/schema/property.py | 28 +++++++++++++++++++++++++++- tests/schema/test_property.py | 21 +++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/ado/schema/property.py b/ado/schema/property.py index f660c6129..2e3dd5d9c 100644 --- a/ado/schema/property.py +++ b/ado/schema/property.py @@ -6,7 +6,7 @@ from typing import Annotated import pydantic -from pydantic import ConfigDict +from pydantic import ConfigDict, ValidationError from ado.schema.domain import PropertyDomain @@ -136,6 +136,32 @@ class Property(pydantic.BaseModel): ] = PropertyDomain() model_config = ConfigDict(frozen=True, extra="forbid") + @pydantic.model_validator(mode="wrap") + @classmethod + def annotate_domain_errors_with_identifier( + cls, + value: typing.Any, # noqa: ANN401 + handler: pydantic.ValidatorFunctionWrapHandler, + ) -> "Property": + """Wrap propertyDomain ValidationErrors to include the property identifier. + + When the propertyDomain is malformed, pydantic raises a ValidationError + that contains no information about which property caused the problem. + This validator intercepts such errors and re-raises them with the + property identifier prepended to the message. + """ + try: + return handler(value) + except ValidationError as exc: + identifier = ( + value.get("identifier") + if isinstance(value, dict) + else getattr(value, "identifier", None) + ) + if identifier: + raise ValueError(f"Property '{identifier}': {exc}") from exc + raise + @classmethod def from_descriptor(cls, descriptor: PropertyDescriptor) -> "Property": diff --git a/tests/schema/test_property.py b/tests/schema/test_property.py index c771f4587..2c4f4f508 100644 --- a/tests/schema/test_property.py +++ b/tests/schema/test_property.py @@ -231,3 +231,24 @@ def test_constitutive_property_identifier_and_string_representation( for t, p in zip(constitutive_property_list, constitutive_properties, strict=True): assert p.identifier == t assert str(p) == t + + +def test_constitutive_property_malformed_domain_error_includes_identifier() -> None: + """When a ConstitutiveProperty has a malformed propertyDomain the ValidationError + message must include the property identifier so the user knows which property is broken. + + This covers the dict-based construction path (used by YAML loading and pydantic's + internal nested-model construction), where pydantic constructs PropertyDomain as a + nested model inside ConstitutiveProperty and ado can therefore annotate the error + with the parent property's identifier. + """ + + with pytest.raises(pydantic.ValidationError, match="total_steps"): + ConstitutiveProperty( + identifier="total_steps", + propertyDomain={ + "variableType": "DISCRETE_VARIABLE_TYPE", + "domainRange": [1, 100_000], + # interval intentionally omitted to trigger the error + }, + ) From ecb3d5b567d4717a9a11876abb811db8f3b80318 Mon Sep 17 00:00:00 2001 From: Alessandro Pomponio Date: Fri, 31 Jul 2026 13:28:48 +0100 Subject: [PATCH 2/3] Revert "feat(core): annotate domain validation errors with property identifier" This reverts commit bfe43a227c529bda550ef355b511f3fdedc120f6. Signed-off-by: Alessandro Pomponio --- ado/schema/property.py | 28 +--------------------------- tests/schema/test_property.py | 21 --------------------- 2 files changed, 1 insertion(+), 48 deletions(-) diff --git a/ado/schema/property.py b/ado/schema/property.py index 2e3dd5d9c..f660c6129 100644 --- a/ado/schema/property.py +++ b/ado/schema/property.py @@ -6,7 +6,7 @@ from typing import Annotated import pydantic -from pydantic import ConfigDict, ValidationError +from pydantic import ConfigDict from ado.schema.domain import PropertyDomain @@ -136,32 +136,6 @@ class Property(pydantic.BaseModel): ] = PropertyDomain() model_config = ConfigDict(frozen=True, extra="forbid") - @pydantic.model_validator(mode="wrap") - @classmethod - def annotate_domain_errors_with_identifier( - cls, - value: typing.Any, # noqa: ANN401 - handler: pydantic.ValidatorFunctionWrapHandler, - ) -> "Property": - """Wrap propertyDomain ValidationErrors to include the property identifier. - - When the propertyDomain is malformed, pydantic raises a ValidationError - that contains no information about which property caused the problem. - This validator intercepts such errors and re-raises them with the - property identifier prepended to the message. - """ - try: - return handler(value) - except ValidationError as exc: - identifier = ( - value.get("identifier") - if isinstance(value, dict) - else getattr(value, "identifier", None) - ) - if identifier: - raise ValueError(f"Property '{identifier}': {exc}") from exc - raise - @classmethod def from_descriptor(cls, descriptor: PropertyDescriptor) -> "Property": diff --git a/tests/schema/test_property.py b/tests/schema/test_property.py index 2c4f4f508..c771f4587 100644 --- a/tests/schema/test_property.py +++ b/tests/schema/test_property.py @@ -231,24 +231,3 @@ def test_constitutive_property_identifier_and_string_representation( for t, p in zip(constitutive_property_list, constitutive_properties, strict=True): assert p.identifier == t assert str(p) == t - - -def test_constitutive_property_malformed_domain_error_includes_identifier() -> None: - """When a ConstitutiveProperty has a malformed propertyDomain the ValidationError - message must include the property identifier so the user knows which property is broken. - - This covers the dict-based construction path (used by YAML loading and pydantic's - internal nested-model construction), where pydantic constructs PropertyDomain as a - nested model inside ConstitutiveProperty and ado can therefore annotate the error - with the parent property's identifier. - """ - - with pytest.raises(pydantic.ValidationError, match="total_steps"): - ConstitutiveProperty( - identifier="total_steps", - propertyDomain={ - "variableType": "DISCRETE_VARIABLE_TYPE", - "domainRange": [1, 100_000], - # interval intentionally omitted to trigger the error - }, - ) From 93a91385e04b698ff1b611f294e691f44ecfb7ce Mon Sep 17 00:00:00 2001 From: Alessandro Pomponio Date: Fri, 31 Jul 2026 13:36:50 +0100 Subject: [PATCH 3/3] feat(core): add contextual information when propertyDomain fails validation Signed-off-by: Alessandro Pomponio --- ado/schema/domain.py | 18 +++++++++++++----- tests/schema/test_domain.py | 4 ++-- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/ado/schema/domain.py b/ado/schema/domain.py index c0a26114c..d5dfb9e40 100644 --- a/ado/schema/domain.py +++ b/ado/schema/domain.py @@ -659,8 +659,12 @@ def variableType_matches_values( valuesCheck = values.data.get("values") is not None intervalCheck = values.data.get("interval") is not None if not (valuesCheck or intervalCheck): + domain_range = values.data.get("domainRange") raise ValueError( - "A DISCRETE_VARIABLE_TYPE had neither values nor interval specified" + f"A DISCRETE_VARIABLE_TYPE had neither values nor interval specified. " + f"Provided: values={values.data.get('values')}, " + f"interval={values.data.get('interval')}, " + f"domainRange={domain_range}" ) elif value == VariableTypeEnum.CONTINUOUS_VARIABLE_TYPE: @@ -677,12 +681,14 @@ def variableType_matches_values( elif value == VariableTypeEnum.OPEN_CATEGORICAL_VARIABLE_TYPE: if values.data.get("interval") is not None: raise ValueError( - "The interval field of an OPEN_CATEGORICAL_VARIABLE_TYPE was not None" + f"An OPEN_CATEGORICAL_VARIABLE_TYPE must not have interval specified. " + f"Provided: interval={values.data.get('interval')}" ) if values.data.get("domainRange") is not None: raise ValueError( - "The domainRange field of an OPEN_CATEGORICAL_VARIABLE_TYPE was not None" + f"An OPEN_CATEGORICAL_VARIABLE_TYPE must not have domainRange specified. " + f"Provided: domainRange={values.data.get('domainRange')}" ) elif value == VariableTypeEnum.BINARY_VARIABLE_TYPE: @@ -700,12 +706,14 @@ def variableType_matches_values( if values.data.get("interval") is not None: raise ValueError( - "The interval field for a BINARY_VARIABLE_TYPE must be None" + f"A BINARY_VARIABLE_TYPE must not have interval specified. " + f"Provided: interval={values.data.get('interval')}" ) if values.data.get("domainRange") is not None: raise ValueError( - "The domainRange field for a BINARY_VARIABLE_TYPE must be None" + f"A BINARY_VARIABLE_TYPE must not have domainRange specified. " + f"Provided: domainRange={values.data.get('domainRange')}" ) return value diff --git a/tests/schema/test_domain.py b/tests/schema/test_domain.py index 16b05fce6..8d5ca043a 100644 --- a/tests/schema/test_domain.py +++ b/tests/schema/test_domain.py @@ -1066,7 +1066,7 @@ def test_binary_variable_type_with_no_values() -> None: def test_binary_variable_type_rejects_interval() -> None: """Test that BINARY_VARIABLE_TYPE rejects interval specification""" with pytest.raises( - ValueError, match="interval field for a BINARY_VARIABLE_TYPE must be None" + ValueError, match="A BINARY_VARIABLE_TYPE must not have interval specified" ): PropertyDomain(variableType=VariableTypeEnum.BINARY_VARIABLE_TYPE, interval=1) @@ -1074,7 +1074,7 @@ def test_binary_variable_type_rejects_interval() -> None: def test_binary_variable_type_rejects_domain_range() -> None: """Test that BINARY_VARIABLE_TYPE rejects domainRange specification""" with pytest.raises( - ValueError, match="domainRange field for a BINARY_VARIABLE_TYPE must be None" + ValueError, match="A BINARY_VARIABLE_TYPE must not have domainRange specified" ): PropertyDomain( variableType=VariableTypeEnum.BINARY_VARIABLE_TYPE, domainRange=[0, 1]