fix: enforce Pydantic constraints on every validation path (review HIGH)

src/metaseed/agent/core.py

@@ -361,9 +361,10 @@ def validate_instance(
         entity_spec = self.get_entity_spec(entity_name)
         errors: list[ValidationIssue] = []
 
-        # Check required fields
+        # Check required fields. A field present with a null value is as absent
+        # as a missing key, so treat both as a missing required field.
         for field in entity_spec.fields:
-            if field.required and field.name not in data:
+            if field.required and (field.name not in data or data[field.name] is None):
                 errors.append(
                     ValidationIssue(
                         field=field.name,

src/metaseed/validators/api.py

@@ -16,6 +16,51 @@
 logger = logging.getLogger(__name__)
 
 
+def _pydantic_constraint_errors(
+    data: dict[str, Any], entity_spec: Any
+) -> list[ValidationError]:
+    """Return Pydantic constraint errors for an entity's simple fields.
+
+    Builds the spec's Pydantic model from the entity's non-nested fields and
+    collects type/pattern/range/length/enum violations. Missing-required errors
+    are skipped here — the engine's RequiredFieldsRule reports those, so this
+    avoids duplicates. Shared by every public validation path so they enforce
+    the same constraints.
+
+    Args:
+        data: Entity data dictionary.
+        entity_spec: The loaded entity spec.
+
+    Returns:
+        List of constraint validation errors (empty if none).
+    """
+    from pydantic import ValidationError as PydanticValidationError
+
+    from metaseed.models.factory import create_model_from_spec
+
+    errors: list[ValidationError] = []
+    try:
+        model_class = create_model_from_spec(entity_spec)
+        nested_field_names = {
+            f.name for f in entity_spec.fields if f.type.value == "list" and f.items
+        }
+        simple_data = {
+            key: value
+            for key, value in data.items()
+            if key not in nested_field_names and not key.startswith("_")
+        }
+        model_class(**simple_data)
+    except PydanticValidationError as e:
+        for err in e.errors():
+            if err["type"] == "missing":
+                continue
+            field_path = ".".join(str(loc) for loc in err["loc"])
+            errors.append(
+                ValidationError(field=field_path, message=err["msg"], rule="constraint")
+            )
+    return errors
+
+
 def _validate_nested(
     data: dict[str, Any],
     entity: str,
@@ -57,6 +102,13 @@ def _validate_nested(
         logger.debug("Could not load entity spec %s: %s", entity, e)
         return errors
 
+    # Pydantic constraint validation (types/patterns/ranges/enums) for this entity.
+    for error in _pydantic_constraint_errors(data, spec):
+        error_field = f"{path}.{error.field}" if path else error.field
+        errors.append(
+            ValidationError(field=error_field, message=error.message, rule=error.rule)
+        )
+
     for field in spec.fields:
         if field.type.value == "list" and field.items:
             items = data.get(field.name, [])
@@ -132,7 +184,15 @@ def validate(
         return _validate_nested(data, entity, version, profile)
 
     engine = create_engine_for_entity(entity, version, profile=profile)
-    return engine.validate(data)
+    errors: list[ValidationError] = list(engine.validate(data))
+    from metaseed.specs.loader import SpecLoader, SpecLoadError
+
+    try:
+        spec = SpecLoader(profile=profile).load_entity(entity, version)
+    except (FileNotFoundError, KeyError, ValueError, SpecLoadError):
+        return errors
+    errors.extend(_pydantic_constraint_errors(data, spec))
+    return errors
 
 
 def validate_entity(
@@ -166,9 +226,6 @@ def validate_entity(
         ...     version="1.2",
         ... )
     """
-    from pydantic import ValidationError as PydanticValidationError
-
-    from metaseed.models.factory import create_model_from_spec
     from metaseed.specs.loader import SpecLoader, SpecLoadError
 
     errors: list[ValidationError] = []
@@ -187,35 +244,7 @@ def validate_entity(
         return errors
 
     # 1. Pydantic validation - checks types, patterns, ranges, etc.
-    try:
-        model_class = create_model_from_spec(entity_spec)
-
-        # Filter to only simple fields (not nested entity lists)
-        nested_field_names = {
-            f.name for f in entity_spec.fields if f.type.value == "list" and f.items
-        }
-        simple_data = {
-            key: value
-            for key, value in data.items()
-            if key not in nested_field_names and not key.startswith("_")
-        }
-
-        model_class(**simple_data)
-
-    except PydanticValidationError as e:
-        for err in e.errors():
-            # Missing required fields are reported by the engine's
-            # RequiredFieldsRule below; skip them here to avoid a duplicate.
-            if err["type"] == "missing":
-                continue
-            field_path = ".".join(str(loc) for loc in err["loc"])
-            errors.append(
-                ValidationError(
-                    field=field_path,
-                    message=err["msg"],
-                    rule="constraint",
-                )
-            )
+    errors.extend(_pydantic_constraint_errors(data, entity_spec))
 
     # 2. Custom rule validation from profile spec
     engine = create_engine_for_entity(entity_type, version, profile=profile)

src/metaseed/validators/dataset.py

@@ -16,6 +16,7 @@
 from metaseed.profiles import ProfileFactory
 from metaseed.specs.loader import SpecLoader, SpecLoadError
 from metaseed.utils import to_snake_case
+from metaseed.validators.api import _pydantic_constraint_errors
 from metaseed.validators.base import ValidationError
 from metaseed.validators.engine import create_engine_for_entity
 
@@ -319,6 +320,20 @@ def validate_node(d: dict[str, Any], etype: str, p: str) -> None:
             except SpecLoadError:
                 pass
 
+            # Pydantic constraint validation (types/patterns/ranges/enums), so the
+            # dataset path enforces the same constraints as the single-entity path.
+            try:
+                spec = SpecLoader(profile=self.profile).load_entity(etype, self.version)
+            except (FileNotFoundError, KeyError, ValueError, SpecLoadError):
+                return
+            for error in _pydantic_constraint_errors(d, spec):
+                field_path = f"{p}.{error.field}" if p else error.field
+                errors.append(
+                    ValidationError(
+                        field=field_path, message=error.message, rule=error.rule
+                    )
+                )
+
         self._traverse_entity_tree(data, entity_type, validate_node, path)
         return errors
 

tests/test_agent/test_core.py

@@ -244,6 +244,13 @@ def test_validate_instance(self) -> None:
             f"Expected error for missing 'unique_id' field, got errors: {errors}"
         )
 
+        # A required field present with a null value is as absent as a missing key.
+        null_data = {"unique_id": None, "title": "Test"}
+        errors = ctx.validate_instance(null_data, "Investigation")
+        assert any(e.field == "unique_id" for e in errors), (
+            f"Expected error for required 'unique_id' set to None, got: {errors}"
+        )
+
     def test_export_yaml(self, tmp_path: Path) -> None:
         """Export extracted data to YAML."""
         csv_file = tmp_path / "data.csv"

tests/test_validators/test_constraint_enforcement.py

@@ -0,0 +1,58 @@
+"""Every public validation path must enforce Pydantic constraints.
+
+Regression guard for the review's HIGH finding: validate() (the REST/CLI path)
+ran only the rule engine, which does not implement pattern/range/length/enum
+constraints, so constraint violations were silently reported as valid while
+validate_entity() (the canonical path) caught them.
+"""
+
+from __future__ import annotations
+
+from metaseed.validators.api import validate, validate_entity
+
+# Study.latitude is constrained to [-90, 90] in the miappe profile.
+_BAD_STUDY = {
+    "unique_id": "ST1",
+    "title": "S",
+    "investigation_id": "I1",
+    "latitude": 999,
+}
+
+
+def _hits_latitude(errors) -> bool:
+    return any("latitude" in e.field for e in errors)
+
+
+def test_validate_entity_catches_range_constraint():
+    assert _hits_latitude(validate_entity(_BAD_STUDY, "Study", version="1.2"))
+
+
+def test_validate_cascade_catches_range_constraint():
+    assert _hits_latitude(validate(_BAD_STUDY, "study", version="1.2", cascade=True))
+
+
+def test_validate_non_cascade_catches_range_constraint():
+    assert _hits_latitude(validate(_BAD_STUDY, "study", version="1.2", cascade=False))
+
+
+def test_valid_study_passes_all_paths():
+    good = {**_BAD_STUDY, "latitude": 45}
+    assert not _hits_latitude(validate_entity(good, "Study", version="1.2"))
+    assert not _hits_latitude(validate(good, "study", version="1.2", cascade=True))
+    assert not _hits_latitude(validate(good, "study", version="1.2", cascade=False))
+
+
+def test_dataset_validator_catches_nested_constraint():
+    """The dataset validation path enforces constraints on nested entities too."""
+    from metaseed.validators.dataset import DatasetValidator
+
+    dv = DatasetValidator(profile="miappe", version="1.2")
+    data = {
+        "unique_id": "I1",
+        "title": "Inv",
+        "studies": [
+            {"unique_id": "S1", "title": "S", "investigation_id": "I1", "latitude": 999}
+        ],
+    }
+    errors = dv._validate_entity(data, "Investigation")
+    assert any("latitude" in e.field for e in errors)