From f503fd06107d47d6ab110137721783b1dac89a35 Mon Sep 17 00:00:00 2001 From: PedroHMCosme Date: Wed, 7 Oct 2026 15:33:15 -0300 Subject: [PATCH] refactor(simple_metrics): extract threshold_breaches helper (SIENTIAPDE-2191) --- laborious/activities/model_metrics.py | 70 ++++++++++--------- .../activities/test_model_metrics.py | 23 +++++- 2 files changed, 60 insertions(+), 33 deletions(-) diff --git a/laborious/activities/model_metrics.py b/laborious/activities/model_metrics.py index 3e7a363..8b59f00 100644 --- a/laborious/activities/model_metrics.py +++ b/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: `_max` breaches when value > threshold, `_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 - # `:`, 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 + # `:`, 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') diff --git a/tests/laborious/activities/test_model_metrics.py b/tests/laborious/activities/test_model_metrics.py index 213f968..e849757 100644 --- a/tests/laborious/activities/test_model_metrics.py +++ b/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