fix the feedback from Mateusz

Signed-off-by: Lukasz Cmielowski <lcmielow@redhat.com>
Assisted-by: Cursor

Author: LukaszCmielowski

components/training/automl/autogluon_leaderboard_evaluation/tests/test_component_unit.py

@@ -1,22 +1,12 @@
 """Tests for the leaderboard_evaluation component."""
 
 import json
-import sys
 from pathlib import Path
 from unittest import mock
 
 import pytest
 
-
-@pytest.fixture(autouse=True, scope="module")
-def isolated_sys_modules():
-    """Patch pandas in sys.modules only for this test module; restored on module teardown."""
-    with mock.patch.dict(sys.modules, clear=False) as mocked_modules:
-        mocked_modules["pandas"] = mock.MagicMock()
-        yield
-
-
-from ..component import leaderboard_evaluation  # noqa: E402
+from ..component import leaderboard_evaluation
 
 
 def _make_models_artifact(
@@ -296,3 +286,71 @@ def test_component_imports_correctly(self):
         assert callable(leaderboard_evaluation)
         assert hasattr(leaderboard_evaluation, "python_func")
         assert hasattr(leaderboard_evaluation, "component_spec")
+
+
+def _write_model_metrics(base_path: Path, model_name: str, metrics: dict) -> None:
+    metrics_dir = base_path / model_name / "metrics"
+    metrics_dir.mkdir(parents=True, exist_ok=True)
+    (metrics_dir / "metrics.json").write_text(json.dumps(metrics), encoding="utf-8")
+
+
+class TestLeaderboardMetricSorting:
+    """Verify AutoGluon negated-metric convention produces correct best-model ranking."""
+
+    def test_negated_rmse_ranks_higher_value_first(self, tmp_path):
+        """Flipped RMSE (-0.3 beats -0.8) selects the better model as best_model."""
+        combined_root = tmp_path / "models"
+        _write_model_metrics(combined_root, "ModelA", {"root_mean_squared_error": -0.8})
+        _write_model_metrics(combined_root, "ModelB", {"root_mean_squared_error": -0.3})
+
+        models_artifact = mock.MagicMock()
+        models_artifact.path = str(combined_root)
+        models_artifact.uri = "http://example.com/artifacts"
+        models_artifact.metadata = {"model_names": json.dumps(["ModelA", "ModelB"])}
+
+        html_artifact = mock.MagicMock()
+        html_artifact.path = str(tmp_path / "leaderboard.html")
+        html_artifact.metadata = {}
+
+        component_status = mock.MagicMock()
+        component_status.path = str(tmp_path / "status")
+        component_status.metadata = {}
+
+        result = leaderboard_evaluation.python_func(
+            models_artifact=models_artifact,
+            eval_metric="root_mean_squared_error",
+            html_artifact=html_artifact,
+            component_status=component_status,
+        )
+
+        assert result.best_model == "ModelB"
+        html = Path(html_artifact.path).read_text(encoding="utf-8")
+        assert html.index("ModelB") < html.index("ModelA")
+
+    def test_mase_ranks_higher_value_first(self, tmp_path):
+        """Timeseries MASE values rank with higher-is-better AutoGluon convention."""
+        combined_root = tmp_path / "models"
+        _write_model_metrics(combined_root, "DeepAR", {"MASE": -0.55})
+        _write_model_metrics(combined_root, "TFT", {"MASE": -0.21})
+
+        models_artifact = mock.MagicMock()
+        models_artifact.path = str(combined_root)
+        models_artifact.uri = "http://example.com/artifacts"
+        models_artifact.metadata = {"model_names": json.dumps(["DeepAR", "TFT"])}
+
+        html_artifact = mock.MagicMock()
+        html_artifact.path = str(tmp_path / "leaderboard_ts.html")
+        html_artifact.metadata = {}
+
+        component_status = mock.MagicMock()
+        component_status.path = str(tmp_path / "status_ts")
+        component_status.metadata = {}
+
+        result = leaderboard_evaluation.python_func(
+            models_artifact=models_artifact,
+            eval_metric="MASE",
+            html_artifact=html_artifact,
+            component_status=component_status,
+        )
+
+        assert result.best_model == "TFT"

components/training/automl/autogluon_leaderboard_evaluation/tests/test_leaderboard_metric_sorting.py

@@ -0,0 +1,11 @@
+"""Expected Kubernetes CPU/memory tiers for the documents indexing pipeline."""
+
+from kfp_components.utils.pipeline_task_resources import ExecutorResources
+
+WORKLOAD_RESOURCES = ExecutorResources("2", "8Gi", "32", "64Gi")
+
+AUTORAG_INDEXING_EXECUTOR_RESOURCES = {
+    "documents-discovery": WORKLOAD_RESOURCES,
+    "text-extraction": WORKLOAD_RESOURCES,
+    "documents-indexing": WORKLOAD_RESOURCES,
+}

pipelines/data_processing/autorag/documents_indexing_pipeline/tests/pipeline_resource_expectations.py

@@ -0,0 +1,21 @@
+"""Unit tests for documents indexing pipeline executor resource tiers."""
+
+from kfp_components.utils.pipeline_task_resources import (
+    assert_executor_resources,
+    compile_executor_resources,
+)
+
+from ..pipeline import documents_indexing_pipeline
+from .pipeline_resource_expectations import AUTORAG_INDEXING_EXECUTOR_RESOURCES
+
+
+class TestDocumentsIndexingPipelineResourceRequirements:
+    """Documents indexing pipeline sets workload-tier resources on all three steps."""
+
+    def test_documents_indexing_pipeline_executor_resources(self):
+        """Documents indexing pipeline sets workload-tier resources on all three steps."""
+        assert_executor_resources(
+            compile_executor_resources(documents_indexing_pipeline),
+            AUTORAG_INDEXING_EXECUTOR_RESOURCES,
+            pipeline_name="documents_indexing_pipeline",
+        )

pipelines/data_processing/autorag/documents_indexing_pipeline/tests/test_pipeline_resource_requirements.py

@@ -116,11 +116,12 @@ def test_compiled_pipeline_wires_s3_and_ogx_secrets(self):
     def test_compiled_pipeline_declares_component_resource_tiers(self):
         """All indexing pipeline steps declare the workload CPU/memory tier."""
         from kfp_components.utils.pipeline_task_resources import (
-            AUTORAG_INDEXING_EXECUTOR_RESOURCES,
             assert_executor_resources,
             compile_executor_resources,
         )
 
+        from .pipeline_resource_expectations import AUTORAG_INDEXING_EXECUTOR_RESOURCES
+
         assert_executor_resources(
             compile_executor_resources(documents_indexing_pipeline),
             AUTORAG_INDEXING_EXECUTOR_RESOURCES,

pipelines/data_processing/autorag/documents_indexing_pipeline/tests/test_pipeline_unit.py

@@ -0,0 +1,18 @@
+"""Expected Kubernetes CPU/memory tiers for the tabular training pipeline."""
+
+from kfp_components.utils.pipeline_task_resources import ExecutorResources
+
+STAGE_MAP_RESOURCES = ExecutorResources("0.5", "512Mi", "1", "1Gi")
+WORKLOAD_RESOURCES = ExecutorResources("2", "8Gi", "32", "64Gi")
+LEADERBOARD_RESOURCES = ExecutorResources("1", "4Gi", "32", "64Gi")
+TRAINING_SPEED_RESOURCES = ExecutorResources("4", "16Gi", "32", "64Gi")
+TRAINING_BALANCED_RESOURCES = ExecutorResources("8", "32Gi", "32", "64Gi")
+
+AUTOML_TABULAR_EXECUTOR_RESOURCES = {
+    "publish-component-stage-map": STAGE_MAP_RESOURCES,
+    "automl-data-loader": WORKLOAD_RESOURCES,
+    "autogluon-models-training": TRAINING_BALANCED_RESOURCES,
+    "leaderboard-evaluation": LEADERBOARD_RESOURCES,
+    "autogluon-models-training-2": TRAINING_SPEED_RESOURCES,
+    "leaderboard-evaluation-2": LEADERBOARD_RESOURCES,
+}

pipelines/training/automl/autogluon_tabular_training_pipeline/tests/pipeline_resource_expectations.py

@@ -0,0 +1,71 @@
+"""Unit tests for pipeline executor resource helpers and tabular tier matrix."""
+
+import pytest
+from kfp_components.utils.pipeline_task_resources import (
+    ExecutorResources,
+    assert_executor_resources,
+    compile_executor_resources,
+    normalize_executor_name,
+)
+
+from ..pipeline import autogluon_tabular_training_pipeline
+from .pipeline_resource_expectations import (
+    AUTOML_TABULAR_EXECUTOR_RESOURCES,
+    TRAINING_BALANCED_RESOURCES,
+    TRAINING_SPEED_RESOURCES,
+)
+
+
+class TestPipelineTaskResourcesHelpers:
+    """Tests for compile/assert helpers in pipeline_task_resources."""
+
+    def test_normalize_executor_name_strips_prefix(self):
+        """Executor keys drop the ``exec-`` prefix for stable task names."""
+        assert normalize_executor_name("exec-automl-data-loader") == "automl-data-loader"
+
+    def test_assert_executor_resources_detects_cpu_change(self):
+        """Mismatched CPU requests fail with the task name in the error."""
+        actual = {
+            "exec-automl-data-loader": ExecutorResources("2", "8Gi", "32", "64Gi"),
+        }
+        expected = {
+            "automl-data-loader": ExecutorResources("4", "8Gi", "32", "64Gi"),
+        }
+        with pytest.raises(AssertionError, match="automl-data-loader"):
+            assert_executor_resources(actual, expected, pipeline_name="test-pipeline")
+
+    def test_assert_executor_resources_allow_extra_executors(self):
+        """Partial expected maps can ignore additional executors when allow_extra is set."""
+        actual = {
+            "exec-automl-data-loader": ExecutorResources("2", "8Gi", "32", "64Gi"),
+            "exec-leaderboard-evaluation": ExecutorResources("1", "4Gi", "32", "64Gi"),
+        }
+        expected = {
+            "automl-data-loader": ExecutorResources("2", "8Gi", "32", "64Gi"),
+        }
+        assert_executor_resources(actual, expected, pipeline_name="test-pipeline", allow_extra=True)
+
+
+class TestAutogluonTabularPipelineResourceRequirements:
+    """Tabular pipeline declares preset-dependent training tiers plus shared loader/leaderboard tiers."""
+
+    def test_tabular_pipeline_executor_resources(self):
+        """All tabular pipeline executors match the declared CPU/memory matrix."""
+        assert_executor_resources(
+            compile_executor_resources(autogluon_tabular_training_pipeline),
+            AUTOML_TABULAR_EXECUTOR_RESOURCES,
+            pipeline_name="autogluon_tabular_training_pipeline",
+        )
+
+    def test_default_speed_preset_uses_lower_training_tier(self):
+        """Default speed preset branch requests less CPU/memory than balanced."""
+        actual = compile_executor_resources(autogluon_tabular_training_pipeline)
+        speed_keys = [name for name in actual if name.endswith("-2") and "models-training" in name]
+        balanced_keys = [name for name in actual if "models-training" in name and not name.endswith("-2")]
+        assert len(speed_keys) == 1
+        assert len(balanced_keys) == 1
+        speed = actual[speed_keys[0]]
+        balanced = actual[balanced_keys[0]]
+        assert speed == TRAINING_SPEED_RESOURCES
+        assert balanced == TRAINING_BALANCED_RESOURCES
+        assert float(speed.cpu_request) < float(balanced.cpu_request)

pipelines/training/automl/autogluon_tabular_training_pipeline/tests/test_pipeline_resource_requirements.py

@@ -175,11 +175,12 @@ def test_compiled_pipeline_wires_preset_to_training_task(self):
     def test_compiled_pipeline_declares_speed_and_balanced_resource_tiers(self):
         """Speed and balanced preset branches request different training CPU/memory."""
         from kfp_components.utils.pipeline_task_resources import (
-            AUTOML_TABULAR_EXECUTOR_RESOURCES,
             assert_executor_resources,
             compile_executor_resources,
         )
 
+        from .pipeline_resource_expectations import AUTOML_TABULAR_EXECUTOR_RESOURCES
+
         actual = compile_executor_resources(autogluon_tabular_training_pipeline)
         assert_executor_resources(
             actual,