refactor(simple_metrics): extract threshold_breaches helper (SIENTIAPDE-2191)

laborious/activities/model_metrics.py

@@ -26,6 +26,28 @@ warnings.filterwarnings(
 )
 
 
+def threshold_breaches(
+    rows: list[dict[str, Any]], thresholds: dict[str, float] | None
+) -> list[str]:
+    """
+    Breach messages for metric rows outside their configured thresholds.
+
+    Convention: `<metric>_max` breaches when value > threshold, `<metric>_min` when value <
+    threshold. Shared by the breach alert and the adaptive-state rule so both agree on what a
+    breach is.
+    """
+    thresholds = thresholds or {}
+    breaches = []
+    for row in rows:
+        metric_name, value = row['metric'], row['value']
+        max_key, min_key = f'{metric_name}_max', f'{metric_name}_min'
+        if max_key in thresholds and value > thresholds[max_key]:
+            breaches.append(f'{metric_name}={value} exceeds {max_key}={thresholds[max_key]}')
+        elif min_key in thresholds and value < thresholds[min_key]:
+            breaches.append(f'{metric_name}={value} below {min_key}={thresholds[min_key]}')
+    return breaches
+
+
 class ModelMetrics(SientiaMonitoring):
     """
     Metrics and statistical analysis activities for the Laborious pipeline.
@@ -430,37 +452,21 @@ class ModelMetrics(SientiaMonitoring):
         self._debug_dataframe(f'Simple metrics dataframe: Size {data.shape}', data, metadata)
 
         # Threshold alerting (optional — no crash when absent)
-        thresholds = input_data.get('thresholds')
-        if thresholds:
-            # Convention: _max thresholds breach when value > threshold,
-            #             _min thresholds breach when value < threshold.
-            for row in output_data:
-                metric_name = row['metric']
-                value = row['value']
-                max_key = f'{metric_name}_max'
-                min_key = f'{metric_name}_min'
-
-                breach_msg = None
-                if max_key in thresholds and value > thresholds[max_key]:
-                    breach_msg = f'{metric_name}={value} exceeds {max_key}={thresholds[max_key]}'
-                elif min_key in thresholds and value < thresholds[min_key]:
-                    breach_msg = f'{metric_name}={value} below {min_key}={thresholds[min_key]}'
-
-                if breach_msg:
-                    message = (
-                        f'Threshold breach for model {model_id} '
-                        f'(window {interval_minutes} min): {breach_msg}'
-                    )
-                    self.warning(message, metadata)
-                    self.send_notification(
-                        # Model-level trigger: the orchestrator dedups alerts on
-                        # `<trigger>:<notification_id>`, so one breach seen by several windows
-                        # (one schedule each) becomes one alert (SIENTIAPDE-1995).
-                        metadata={**metadata, 'schedule_name': f'{model_id}-simple_metrics'},
-                        notification_id='SIMPLE_METRICS_THRESHOLD_BREACH',
-                        message=message,
-                        block='model_metrics',
-                        level=NotificationLevel.WARNING,
-                    )
+        for breach_msg in threshold_breaches(output_data, input_data.get('thresholds')):
+            message = (
+                f'Threshold breach for model {model_id} '
+                f'(window {interval_minutes} min): {breach_msg}'
+            )
+            self.warning(message, metadata)
+            self.send_notification(
+                # Model-level trigger: the orchestrator dedups alerts on
+                # `<trigger>:<notification_id>`, so one breach seen by several windows
+                # (one schedule each) becomes one alert (SIENTIAPDE-1995).
+                metadata={**metadata, 'schedule_name': f'{model_id}-simple_metrics'},
+                notification_id='SIMPLE_METRICS_THRESHOLD_BREACH',
+                message=message,
+                block='model_metrics',
+                level=NotificationLevel.WARNING,
+            )
 
         return data.to_dict(orient='records')

tests/laborious/activities/test_model_metrics.py

@@ -7,7 +7,7 @@ from sientia_do.observability.metrics_controller import MetricsController
 from sientia_do.temporal.constants import DATETIME_FORMAT_WITH_TZ
 from sientia_model.analytics.drift_analysis import DriftInsufficientDataError
 
-from laborious.activities.model_metrics import ModelMetrics
+from laborious.activities.model_metrics import ModelMetrics, threshold_breaches
 
 
 @fixture
@@ -1221,3 +1221,24 @@ def test_to_naive_utc_keeps_naive_timestamps():
 
     assert result.dt.tz is None
     assert result.tolist() == [Timestamp('2024-01-01 06:00:00')]
+
+
+@mark.parametrize(
+    'rows, thresholds, expected_count',
+    [
+        ([{'metric': 'rmse', 'value': 2.0}], {'rmse_max': 1.0}, 1),
+        ([{'metric': 'rmse', 'value': 0.5}], {'rmse_max': 1.0}, 0),
+        ([{'metric': 'r2', 'value': 0.5}], {'r2_min': 0.9}, 1),
+        ([{'metric': 'r2', 'value': 0.95}], {'r2_min': 0.9}, 0),
+        ([{'metric': 'mae', 'value': 99.0}], {'rmse_max': 1.0}, 0),
+        ([{'metric': 'rmse', 'value': 99.0}], None, 0),
+        ([], {'rmse_max': 1.0}, 0),
+        (
+            [{'metric': 'rmse', 'value': 2.0}, {'metric': 'r2', 'value': 0.1}],
+            {'rmse_max': 1.0, 'r2_min': 0.9},
+            2,
+        ),
+    ],
+)
+def test_threshold_breaches(rows, thresholds, expected_count):
+    assert len(threshold_breaches(rows, thresholds)) == expected_count