feat(forms): show metadata validation errors under each field

bec_widgets/utils/forms_from_types/forms.py

@@ -1,20 +1,12 @@
 from __future__ import annotations
 
 from types import GenericAlias, NoneType, UnionType
-from typing import NamedTuple
+from typing import NamedTuple, Union, get_args, get_origin
 
 from bec_lib.logger import bec_logger
 from pydantic import BaseModel, ValidationError
-from qtpy.QtCore import Signal  # type: ignore
-from qtpy.QtWidgets import (
-    QApplication,
-    QGridLayout,
-    QHBoxLayout,
-    QLabel,
-    QSizePolicy,
-    QVBoxLayout,
-    QWidget,
-)
+from qtpy.QtCore import Qt, Signal  # type: ignore
+from qtpy.QtWidgets import QApplication, QGridLayout, QLabel, QSizePolicy, QVBoxLayout, QWidget
 
 from bec_widgets.utils.bec_widget import BECWidget
 from bec_widgets.utils.colors import get_accent_colors
@@ -36,6 +28,15 @@ class GridRow(NamedTuple):
     widget: DynamicFormItem
 
 
+class ValidationIssue(NamedTuple):
+    """One invalid field of a form, ready for display."""
+
+    field: str
+    title: str
+    problem: str
+    entered: str
+
+
 class TypedForm(BECWidget, QWidget):
     PLUGIN = True
     ICON_NAME = "list_alt"
@@ -129,7 +130,10 @@ class TypedForm(BECWidget, QWidget):
         which the field name is attached as a property "_model_field_name"), and the entry widget"""
         grid: QGridLayout = self._form_grid.layout()  # type: ignore
         for i in range(grid.rowCount() - 1):  # One extra row for stretch
-            yield GridRow(i, grid.itemAtPosition(i, 0).widget(), grid.itemAtPosition(i, 1).widget())
+            label, widget = grid.itemAtPosition(i, 0), grid.itemAtPosition(i, 1)
+            if label is None or widget is None:  # e.g. the error line under a field
+                continue
+            yield GridRow(i, label.widget(), widget.widget())
 
     def _dict_from_grid(self) -> dict[str, DynamicFormItemType]:
         return {
@@ -203,6 +207,7 @@ class PydanticModelForm(TypedForm):
         """
         self._pretty_display = pretty_display
         self._md_schema = data_model
+        self._field_errors: dict[str, QLabel] = {}
         super().__init__(
             parent=parent,
             form_item_specs=self._form_item_specs(),
@@ -211,13 +216,13 @@ class PydanticModelForm(TypedForm):
             **kwargs,
         )
 
-        self._validity = QWidget(self)
-        validity_layout = QHBoxLayout(self._validity)
-        validity_layout.setContentsMargins(0, 0, 0, 0)
-        self._validity_message = QLabel("Not yet validated", self._validity)
+        self._validity_message = QLabel("Not yet validated", self)
         self._validity_message.setWordWrap(True)
-        validity_layout.addWidget(self._validity_message)
+        self._validity_message.setTextFormat(Qt.TextFormat.PlainText)
+        self._validity = self._validity_message
         self._validity_state: bool | None = None
+        self._validity_summary = "Not yet validated"
+        self._validity_issues: list[ValidationIssue] = []
         self._layout.addWidget(self._validity)
         self.value_changed.connect(self.validate_form)
 
@@ -266,19 +271,106 @@ class PydanticModelForm(TypedForm):
 
     def populate(self):
         self.update_items_from_schema()
+        self._field_errors = {}
         super().populate()
 
+    def _add_griditem(self, item: FormItemSpec, row: int):
+        # every field takes two grid rows: the field itself and its (hidden) error line below
+        super()._add_griditem(item, 2 * row)
+        error = QLabel(parent=self._form_grid)
+        error.setWordWrap(True)
+        error.setVisible(False)
+        self._form_grid.layout().addWidget(error, 2 * row + 1, 1)
+        self._field_errors[item.name] = error
+
     def get_form_data(self):
         """Get the entered metadata as a dict."""
         return self._dict_from_grid()
 
-    def _set_validity(self, valid: bool, message: str):
-        """Show the validation result inline, coloured by the theme's accent colours."""
+    def _set_validity(self, valid: bool, message: str, details: str = ""):
+        """Show a short validation result under the form.
+
+        Args:
+            valid (bool): whether the form data is valid.
+            message (str): the one-line summary.
+            details (str): readable list of the problems, used as tooltip.
+        """
         self._validity_state = valid
+        self._validity_summary = message
         accents = get_accent_colors()
         color = accents.success if valid else accents.emergency
         self._validity_message.setStyleSheet(f"color: {color.name()};")
         self._validity_message.setText(message)
+        self._validity_message.setToolTip(details)
+
+    def _show_field_errors(self, issues: list[ValidationIssue]):
+        """Show each field's problem directly under its input; hide the lines of valid fields."""
+        problems = {issue.field: issue.problem for issue in issues}
+        color = get_accent_colors().emergency.name()
+        for name, label in self._field_errors.items():
+            problem = problems.get(name)
+            label.setVisible(problem is not None)
+            if problem is not None:
+                label.setStyleSheet(f"color: {color};")
+                label.setText(problem)
+
+    def _describe_errors(self, error: ValidationError) -> list[ValidationIssue]:
+        """One issue per invalid top-level field, without pydantic's error codes and links.
+
+        Errors inside a field (list entries, dict keys, nested models) keep their location, e.g.
+        ``[1]: Input should be a valid integer``; every failing entry of a field is listed."""
+        problems: dict[str, list[str]] = {}
+        entered: dict[str, str] = {}
+        for err in error.errors():
+            loc = tuple(err["loc"])
+            name = str(loc[0]) if loc else "form"
+            path = self._error_path(name, loc[1:])
+            value = err.get("input")
+            if err["type"] == "missing" or value is None:
+                message = "Required - please fill in"
+            else:
+                message = err["msg"]
+            line = f"{path}: {message}" if path else message
+            lines = problems.setdefault(name, [])
+            if line not in lines:
+                lines.append(line)
+            if name not in entered:
+                if value is None:
+                    entered[name] = "empty"
+                else:
+                    entered[name] = repr(value) if isinstance(value, str) else str(value)
+        issues = []
+        for name, lines in problems.items():
+            field = self._md_schema.model_fields.get(name)
+            title = field.title if field is not None and field.title else name
+            issues.append(ValidationIssue(name, title, "\n".join(lines), entered[name]))
+        return issues
+
+    def _error_path(self, name: str, sub_loc: tuple) -> str:
+        """Readable location inside a field, e.g. ``[1]``, ``key`` or ``inner[0].name``.
+
+        For fields with several possible types (``int | float``), pydantic reports one error per
+        type, tagged with the type name; that tag is not a location and is dropped."""
+        field = self._md_schema.model_fields.get(name)
+        annotation = field.annotation if field is not None else None
+        if get_origin(annotation) in (Union, UnionType):
+            members = [arg for arg in get_args(annotation) if arg is not NoneType]
+            if len(members) > 1 and sub_loc and isinstance(sub_loc[0], str):
+                sub_loc = sub_loc[1:]
+        path = ""
+        for part in sub_loc:
+            path += f"[{part}]" if isinstance(part, int) else f".{part}"
+        return path.lstrip(".")
+
+    @staticmethod
+    def _summarize_issues(issues: list[ValidationIssue]) -> str:
+        if len(issues) == 1:
+            return f"Invalid field: {issues[0].title}"
+        return f"{len(issues)} invalid fields: {', '.join(issue.title for issue in issues)}"
+
+    def _summarize_errors(self, error: ValidationError) -> str:
+        """One short line naming the invalid fields, so the form keeps its size."""
+        return self._summarize_issues(self._describe_errors(error))
 
     def validate_form(self, *_) -> bool:
         """validate the currently entered metadata against the pydantic schema.
@@ -287,12 +379,21 @@ class PydanticModelForm(TypedForm):
         try:
             metadata_dict = self.get_form_data()
             self._md_schema.model_validate(metadata_dict)
+            self._validity_issues = []
+            self._show_field_errors([])
             self._set_validity(True, "No errors!")
             self.form_data_updated.emit(metadata_dict)
             self.validity_proc.emit(True)
             return True
         except ValidationError as e:
-            self._set_validity(False, str(e))
+            self._validity_issues = self._describe_errors(e)
+            self._show_field_errors(self._validity_issues)
+            details = "\n".join(
+                f"{issue.title}: {line}"
+                for issue in self._validity_issues
+                for line in issue.problem.split("\n")
+            )
+            self._set_validity(False, self._summarize_issues(self._validity_issues), details)
             self.form_data_cleared.emit(None)
             self.validity_proc.emit(False)
             return False

bec_widgets/widgets/services/device_browser/device_item/device_config_dialog.py

@@ -135,7 +135,7 @@ class DeviceConfigDialog(QDialog):
         # TODO: replace when https://github.com/bec-project/bec/issues/528 https://github.com/bec-project/bec/issues/547 are resolved
         for button in [self.button_box.button(b) for b in [_StdBtn.Apply, _StdBtn.Ok]]:
             button.setEnabled(valid)
-            button.setToolTip(self._form._validity_message.text())
+            button.setToolTip(self._form._validity_message.toolTip())
 
     def _process_action(self):
         self.accepted_data.emit(self._form.get_form_data())

tests/end-2-end/test_with_plugins_e2e.py

@@ -7,7 +7,6 @@ try:
 except ImportError:
     pytest.skip(reason="Requires plugin repo!", allow_module_level=True)
 
-from qtpy.QtWidgets import QGridLayout
 
 from bec_widgets.utils.widget_io import WidgetIO
 from bec_widgets.widgets.control.scan_control import ScanControl
@@ -68,11 +67,9 @@ def test_scan_metadata_for_custom_scan(
 
     def do_test():
         # Set the metadata
-        grid: QGridLayout = scan_control._metadata_form._form_grid.layout()
-        for i in range(grid.rowCount() - 1):  # type: ignore
-            field_name = grid.itemAtPosition(i, 0).widget().property("_model_field_name")
+        for field_name, widget in scan_control._metadata_form.widget_dict.items():
             if (value_to_set := md.pop(field_name, None)) is not None:
-                grid.itemAtPosition(i, 1).widget().setValue(value_to_set)
+                widget.setValue(value_to_set)
         # all values should be used
         assert md == {}
         assert scan_control.button_run_scan.isEnabled()

tests/unit_tests/test_scan_control.py

@@ -1772,7 +1772,7 @@ def test_scan_metadata_is_connected(scan_control):
     assert scan_control._metadata_form._scan_name == "line_scan"
     scan_control.comboBox_scan_selection.setCurrentText("grid_scan")
     assert scan_control._metadata_form._scan_name == "grid_scan"
-    sample_name = scan_control._metadata_form._form_grid.layout().itemAtPosition(2, 1).widget()
+    sample_name = scan_control._metadata_form.widget_dict["sample_name"]
     assert isinstance(sample_name, StrFormItem)
     sample_name._main_widget.setText("Test Sample")
 
@@ -1784,7 +1784,7 @@ def test_scan_metadata_is_connected(scan_control):
 def test_scan_metadata_is_passed_to_scan_function(scan_control: ScanControl):
     scan_control.comboBox_scan_selection.setCurrentText("grid_scan")
 
-    sample_name = scan_control._metadata_form._form_grid.layout().itemAtPosition(2, 1).widget()
+    sample_name = scan_control._metadata_form.widget_dict["sample_name"]
     sample_name._main_widget.setText("Test Sample")
     scan_control._metadata_form._additional_metadata._table_model._data = TEST_TABLE_ENTRY
     scan_control._metadata_form.validate_form()

tests/unit_tests/test_scan_metadata.py

@@ -3,7 +3,7 @@ from typing import Set
 
 import pytest
 from bec_lib.metadata_schema import BasicScanMetadata
-from pydantic import Field
+from pydantic import BaseModel, Field, ValidationError
 from pydantic.types import Json
 from qtpy.QtCore import QItemSelectionModel, QPoint, Qt
 
@@ -77,40 +77,7 @@ def metadata_widget(empty_metadata_widget: ScanMetadata):
     widget._md_schema = ExampleSchema
     widget.populate()
 
-    scan_name = widget._form_grid.layout().itemAtPosition(0, 1).widget()
-    comment = widget._form_grid.layout().itemAtPosition(1, 1).widget()
-    sample_name = widget._form_grid.layout().itemAtPosition(2, 1).widget()
-    str_optional = widget._form_grid.layout().itemAtPosition(3, 1).widget()
-    str_required = widget._form_grid.layout().itemAtPosition(4, 1).widget()
-    bool_optional = widget._form_grid.layout().itemAtPosition(5, 1).widget()
-    bool_required_default = widget._form_grid.layout().itemAtPosition(6, 1).widget()
-    bool_required_nodefault = widget._form_grid.layout().itemAtPosition(7, 1).widget()
-    int_default = widget._form_grid.layout().itemAtPosition(8, 1).widget()
-    int_nodefault_optional = widget._form_grid.layout().itemAtPosition(9, 1).widget()
-    float_nodefault = widget._form_grid.layout().itemAtPosition(10, 1).widget()
-    decimal_dp_limits_nodefault = widget._form_grid.layout().itemAtPosition(11, 1).widget()
-    dict_default = widget._form_grid.layout().itemAtPosition(12, 1).widget()
-    unsupported_class = widget._form_grid.layout().itemAtPosition(13, 1).widget()
-
-    yield (
-        widget,
-        {
-            "scan_name": scan_name,
-            "comment": comment,
-            "sample_name": sample_name,
-            "str_optional": str_optional,
-            "str_required": str_required,
-            "bool_optional": bool_optional,
-            "bool_required_default": bool_required_default,
-            "bool_required_nodefault": bool_required_nodefault,
-            "int_default": int_default,
-            "int_nodefault_optional": int_nodefault_optional,
-            "float_nodefault": float_nodefault,
-            "decimal_dp_limits_nodefault": decimal_dp_limits_nodefault,
-            "dict_default": dict_default,
-            "unsupported_class": unsupported_class,
-        },
-    )
+    yield widget, widget.widget_dict
 
 
 def fill_components(components: dict[str, DynamicFormItem]):
@@ -166,15 +133,171 @@ def test_validation(metadata_widget: tuple[ScanMetadata, dict[str, DynamicFormIt
     components["bool_required_nodefault"]._main_widget.clear()
     widget.validate_form()
     assert widget._validity_state is False
-    assert "Input should be a valid boolean" in widget._validity_message.text()
+    assert widget._validity_message.text() == "Invalid field: bool_required_nodefault"
+    assert (
+        widget._validity_message.toolTip() == "bool_required_nodefault: Required - please fill in"
+    )
     components["bool_required_nodefault"].setValue(True)
 
     components["float_nodefault"]._main_widget.clear()
     widget.validate_form()
-    assert "Input should be a valid number" in widget._validity_message.text()
+    assert widget._validity_message.text() == "Invalid field: float_nodefault"
+    assert widget._validity_message.toolTip() == "float_nodefault: Required - please fill in"
     components["float_nodefault"].setValue(True)
 
 
+def test_validation_summary_stays_short(
+    metadata_widget: tuple[ScanMetadata, dict[str, DynamicFormItem]],
+):
+    widget, components = metadata_widget
+    fill_components(components)
+    components["bool_required_nodefault"]._main_widget.clear()
+    components["float_nodefault"]._main_widget.clear()
+    widget.validate_form()
+    summary = widget._validity_summary
+    assert summary == "2 invalid fields: bool_required_nodefault, float_nodefault"
+    assert "\n" not in summary
+    assert "errors.pydantic.dev" not in widget._validity_message.toolTip()
+
+
+def test_validation_summary_uses_field_titles(
+    metadata_widget: tuple[ScanMetadata, dict[str, DynamicFormItem]],
+):
+    widget, _ = metadata_widget
+    with pytest.raises(ValidationError) as excinfo:
+        ExampleSchema.model_validate(TEST_DICT | {"str_optional": "x" * 30})
+    assert "Optional string" in widget._summarize_errors(excinfo.value)
+
+
+def test_validation_summary_groups_union_and_list_errors(
+    metadata_widget: tuple[ScanMetadata, dict[str, DynamicFormItem]],
+):
+    class UnionListSchema(BasicScanMetadata):
+        u: int | float = Field(title="Union")
+        lst: list[int] = Field(default_factory=list, title="List")
+
+    widget, _ = metadata_widget
+    with pytest.raises(ValidationError) as excinfo:
+        UnionListSchema.model_validate({"u": "x", "lst": [1, "a", "b"]})
+    widget._md_schema = UnionListSchema
+    assert widget._summarize_errors(excinfo.value) == "2 invalid fields: Union, List"
+
+
+def test_validation_issues_are_readable(
+    metadata_widget: tuple[ScanMetadata, dict[str, DynamicFormItem]],
+):
+    class ReadableSchema(BasicScanMetadata):
+        name: str = Field(max_length=3, title="Name")
+        count: int = Field(title="Count")
+        value: Decimal = Field(decimal_places=1, title="Value")
+
+    widget, _ = metadata_widget
+    widget._md_schema = ReadableSchema
+    with pytest.raises(ValidationError) as excinfo:
+        ReadableSchema.model_validate({"name": "toolong", "count": None, "value": Decimal("1.25")})
+    issues = widget._describe_errors(excinfo.value)
+    assert [(i.field, i.title, i.entered) for i in issues] == [
+        ("name", "Name", "'toolong'"),
+        ("count", "Count", "empty"),
+        ("value", "Value", "1.25"),
+    ]
+    assert issues[0].problem == "String should have at most 3 characters"
+    assert issues[1].problem == "Required - please fill in"
+
+
+def test_validation_issues_keep_nested_locations(
+    metadata_widget: tuple[ScanMetadata, dict[str, DynamicFormItem]],
+):
+    class Detector(BaseModel):
+        prefix: str
+        gain: int
+
+    class NestedSchema(BasicScanMetadata):
+        positions: list[int] = Field(default_factory=list, title="Positions")
+        offsets: dict[str, int] = Field(default_factory=dict, title="Offsets")
+        detector: Detector | None = Field(None, title="Detector")
+        value: int | float = Field(0, title="Value")
+
+    widget, _ = metadata_widget
+    widget._md_schema = NestedSchema
+    with pytest.raises(ValidationError) as excinfo:
+        NestedSchema.model_validate(
+            {
+                "positions": [1, "a", 3, "b"],
+                "offsets": {"x": 1, "y": "bad"},
+                "detector": {"gain": "hi"},
+                "value": "x",
+            }
+        )
+    issues = {issue.field: issue for issue in widget._describe_errors(excinfo.value)}
+    positions = issues["positions"].problem.split("\n")
+    assert [line.split(":")[0] for line in positions] == ["[1]", "[3]"]
+    assert issues["offsets"].problem.startswith("y: Input should be a valid integer")
+    assert issues["detector"].problem.split("\n") == [
+        "prefix: Required - please fill in",
+        "gain: Input should be a valid integer, unable to parse string as an integer",
+    ]
+    # union members are tagged with their type name, which is not a location
+    assert not any(
+        line.startswith(("int:", "float:")) for line in issues["value"].problem.split("\n")
+    )
+    assert len(issues["value"].problem.split("\n")) == 2
+
+
+def test_validation_errors_under_each_field(
+    metadata_widget: tuple[ScanMetadata, dict[str, DynamicFormItem]],
+):
+    widget, components = metadata_widget
+    errors = widget._field_errors
+    assert set(errors) == set(components)
+    assert not any(label.isVisibleTo(widget) for label in errors.values())
+
+    fill_components(components)
+    components["bool_required_nodefault"]._main_widget.clear()
+    components["float_nodefault"]._main_widget.clear()
+    widget.validate_form()
+    shown = {name for name, label in errors.items() if label.isVisibleTo(widget)}
+    assert shown == {"bool_required_nodefault", "float_nodefault"}
+    assert errors["float_nodefault"].text() == "Required - please fill in"
+    assert "errors.pydantic.dev" not in errors["float_nodefault"].text()
+
+    components["bool_required_nodefault"].setValue(True)
+    widget.validate_form()
+    shown = {name for name, label in errors.items() if label.isVisibleTo(widget)}
+    assert shown == {"float_nodefault"}
+
+    components["float_nodefault"].setValue(1.5)
+    widget.validate_form()
+    assert not any(label.isVisibleTo(widget) for label in errors.values())
+
+
+def test_validation_error_line_sits_under_its_field(
+    metadata_widget: tuple[ScanMetadata, dict[str, DynamicFormItem]],
+):
+    widget, components = metadata_widget
+    grid = widget._form_grid.layout()
+    rows = list(widget.enumerate_form_widgets())
+    assert len(rows) == len(components)  # error lines are not form rows
+    for row in rows:
+        name = row.label.property("_model_field_name")
+        index = grid.indexOf(widget._field_errors[name])
+        assert grid.getItemPosition(index)[:2] == (row.i + 1, 1)
+
+
+def test_validation_error_lines_follow_schema_change(
+    metadata_widget: tuple[ScanMetadata, dict[str, DynamicFormItem]],
+):
+    class OtherSchema(BasicScanMetadata):
+        count: int = Field(title="Count")
+
+    widget, _ = metadata_widget
+    widget.set_schema(OtherSchema)
+    assert set(widget._field_errors) == {"scan_name", "comment", "sample_name", "count"}
+    widget.validate_form()
+    assert widget._field_errors["count"].isVisibleTo(widget)
+    assert widget._validity_message.text() == "Invalid field: Count"
+
+
 def test_numbers_clipped_to_limits(
     metadata_widget: tuple[ScanMetadata, dict[str, DynamicFormItem]],
 ):