Fixes for copilot review of monitoring code

Author: verveerpj

src/everest/bin/utils.py

@@ -261,30 +261,31 @@ def _get_opt_progress_single_batch(self, cli_monitor_data: dict[str, Any]) -> st
 
         lines = [self._make_header(f"Optimization progress (Batch #{batch})")]
 
-        if controls := cli_monitor_data.get("controls"):
-            width = _get_max_width(controls.keys())
-            lines.append(
-                self._join_one_newline_indent(
-                    [
-                        f"{name:>{width}}: {value:{self.FLOAT_FMT}}"
-                        for name, value in controls.items()
-                    ]
+        if cli_monitor_data.get("result_type") == "FunctionResult":
+            if controls := cli_monitor_data.get("controls"):
+                width = _get_max_width(controls.keys())
+                lines.append(
+                    self._join_one_newline_indent(
+                        [
+                            f"{name:>{width}}: {value:{self.FLOAT_FMT}}"
+                            for name, value in controls.items()
+                        ]
+                    )
                 )
-            )
-        if expected_objectives := cli_monitor_data.get("expected_objectives"):
-            width = _get_max_width(expected_objectives.keys())
-            lines.append(
-                self._join_one_newline_indent(
-                    [
-                        f"{name:>{width}}: {value:{self.FLOAT_FMT}}"
-                        for name, value in expected_objectives.items()
-                    ]
+            if expected_objectives := cli_monitor_data.get("expected_objectives"):
+                width = _get_max_width(expected_objectives.keys())
+                lines.append(
+                    self._join_one_newline_indent(
+                        [
+                            f"{name:>{width}}: {value:{self.FLOAT_FMT}}"
+                            for name, value in expected_objectives.items()
+                        ]
+                    )
+                )
+            if objective_value := cli_monitor_data.get("objective_value"):
+                lines.append(
+                    f"Total normalized objective: {objective_value:{self.FLOAT_FMT}}"
                 )
-            )
-        if objective_value := cli_monitor_data.get("objective_value"):
-            lines.append(
-                f"Total normalized objective: {objective_value:{self.FLOAT_FMT}}"
-            )
 
         if failures := cli_monitor_data.get("failures", {}):
             failed_lines = []

src/everest/detached/client.py

@@ -211,24 +211,20 @@ def server_is_running(url: str, cert: str, auth: tuple[str, str]) -> bool:
 def get_opt_status_from_batch_result_event(
     event: EverestBatchResultEvent,
 ) -> dict[str, Any]:
-    if not event.results:
-        return {}
-
-    assert event.batch is not None
-
-    if event.result_type == "FunctionResult":
-        return {
-            "batch": event.batch,
-            "controls": event.results["controls"],
-            "objective_value": event.results["total_objective_value"],
-            "expected_objectives": event.results["objectives"],
-            "failures": event.failures,
-        }
-
-    return {
+    status = {
+        "result_type": event.result_type,
         "batch": event.batch,
         "failures": event.failures,
     }
+    if event.results and event.result_type == "FunctionResult":
+        status.update(
+            {
+                "controls": event.results["controls"],
+                "objective_value": event.results["total_objective_value"],
+                "expected_objectives": event.results["objectives"],
+            }
+        )
+    return status
 
 
 def start_monitor(

tests/everest/test_monitor.py

@@ -177,7 +177,7 @@ def snapshot_update_event_with_fm_message():
 
 
 @pytest.mark.slow
-def test_failed_jobs_monitor(
+def test_that_the_monitor_shows_failed_jobs(
     monkeypatch, full_snapshot_event, snapshot_update_failure_event, capsys
 ):
     server_mock = MagicMock()
@@ -215,7 +215,9 @@ def test_failed_jobs_monitor(
 
 
 @pytest.mark.slow
-def test_monitor(monkeypatch, full_snapshot_event, snapshot_update_event, capsys):
+def test_that_the_monitor_shows_running_jobs(
+    monkeypatch, full_snapshot_event, snapshot_update_event, capsys
+):
     server_mock = MagicMock()
     connection_mock = MagicMock(spec=ClientConnection)
     connection_mock.recv.side_effect = [
@@ -251,7 +253,7 @@ def test_monitor(monkeypatch, full_snapshot_event, snapshot_update_event, capsys
 
 
 @pytest.mark.slow
-def test_forward_model_message_reaches_the_cli(
+def test_that_a_forward_model_message_reaches_the_cli(
     monkeypatch, full_snapshot_event, snapshot_update_event_with_fm_message, capsys
 ):
     server_mock = MagicMock()
@@ -264,10 +266,11 @@ def test_forward_model_message_reaches_the_cli(
     server_mock.return_value.__enter__.return_value = connection_mock
     monkeypatch.setattr(everest.detached.client, "connect", server_mock)
     monkeypatch.setattr(everest.detached.client, "ssl", MagicMock())
-    partial(everest.detached.start_monitor, polling_interval=0.1)
-    run_detached_monitor(
-        ("some/url", "cert", ("username", "password")), run_id="test-run-id"
-    )
+    patched = partial(everest.detached.start_monitor, polling_interval=0.1)
+    with patch("everest.bin.utils.start_monitor", patched):
+        run_detached_monitor(
+            ("some/url", "cert", ("username", "password")), run_id="test-run-id"
+        )
     captured = capsys.readouterr()
 
     expected = [
@@ -290,7 +293,7 @@ def test_forward_model_message_reaches_the_cli(
 
 
 @pytest.mark.slow
-def test_failed_everest_batch_result_event(
+def test_that_a_failed_everest_batch_result_event_is_shown(
     monkeypatch, everest_batch_result_event, capsys
 ):
     server_mock = MagicMock()
@@ -302,10 +305,11 @@ def test_failed_everest_batch_result_event(
     server_mock.return_value.__enter__.return_value = connection_mock
     monkeypatch.setattr(everest.detached.client, "connect", server_mock)
     monkeypatch.setattr(everest.detached.client, "ssl", MagicMock())
-    partial(everest.detached.start_monitor, polling_interval=0.1)
-    run_detached_monitor(
-        ("some/url", "cert", ("username", "password")), run_id="test-run-id"
-    )
+    patched = partial(everest.detached.start_monitor, polling_interval=0.1)
+    with patch("everest.bin.utils.start_monitor", patched):
+        run_detached_monitor(
+            ("some/url", "cert", ("username", "password")), run_id="test-run-id"
+        )
     captured = capsys.readouterr()
     expected = [
         "============= Optimization progress (Batch #0) =============\n",