Do not report a printer as Safe when nothing is checking it (issue #2952)

The printer card's AI badge collapsed every class that was not Warning or
    Failure into green Safe, and the service reported `safe` whenever it had no
    verdict. The state entry is created when a monitored print is first seen --
    before the first snapshot, let alone the first inference -- so a rejected ML
    API token, an unreachable ML API, a failed capture and an unset External URL
    all rendered as a healthy watched print: green Safe at score 0.000.

    For a safety feature that is the worst failure mode available: it asserts the
    print is being watched exactly when it is not. The reporter read that badge and
    concluded the loop had never started. It had been calling the ML API every ten
    seconds and being turned away with a 401 -- invisible because Obico's auth
    layer rejects a bad token before its request log sees it, and because
    successful checks log nothing there either.

    Add two honest states. Not checking (amber) when the last poll produced no
    result, carrying the reason; Starting while a monitored print waits for its
    first result. Score and frame count are withheld while not checking, since
    0.000 beside "Not checking" reads as a measurement rather than its absence.
    The reason is per printer, so a card names its own problem rather than
    whichever printer failed most recently, and stays behind settings:read because
    it can quote configured URLs -- the badge state does not, because whether a
    print is watched is not configuration. An unrecognised class now falls back to
    Starting, not Safe.

    Test Connection saves the form before probing, so a green result describes the
    configuration the loop actually runs with rather than what is typed in the
    boxes.

backend/app/api/routes/obico.py

@@ -53,11 +53,17 @@ async def get_printer_status(
     # Error strings can embed configured URLs (ML API base, external URL), so
     # they stay behind settings:read like the rest of the configuration.
     can_see_error = user is None or user.has_permission(Permission.SETTINGS_READ.value)
+    per_printer = obico_detection_service.get_per_printer()
+    if not can_see_error:
+        # The "error" *class* is not configuration — a printers:read user still
+        # needs to know their print is not being watched. Only the reason, which
+        # can name a URL, is withheld.
+        per_printer = {pid: {**entry, "error": None} for pid, entry in per_printer.items()}
     return {
         "enabled": settings["enabled"],
         # None = all printers are monitored
         "monitored_printers": sorted(enabled_printers) if enabled_printers is not None else None,
-        "per_printer": obico_detection_service.get_per_printer(),
+        "per_printer": per_printer,
         "last_error": obico_detection_service._last_error if can_see_error else None,
     }
 

backend/app/services/obico_detection.py

@@ -96,8 +96,14 @@ class ObicoDetectionService:
         self._states: dict[int, PrintState] = {}
         # printer_id -> task_name active when state was created (used to detect new prints)
         self._state_keys: dict[int, str] = {}
-        # printer_id -> last classification ("safe"/"warning"/"failure")
+        # printer_id -> last classification ("safe"/"warning"/"failure").
+        # Only written after an inference actually came back, so a missing entry
+        # means "we have no verdict", which is not the same as "safe" (#2952).
         self._last_class: dict[int, str] = {}
+        # printer_id -> why the most recent poll produced no verdict, or absent
+        # when the last poll succeeded. Per-printer rather than global so a card
+        # can say what went wrong for *that* printer.
+        self._errors: dict[int, str] = {}
         # printer_id -> whether an action has already been fired for the current print
         self._action_fired: dict[int, bool] = {}
         # Global detection event log (most-recent-first)
@@ -191,6 +197,8 @@ class ObicoDetectionService:
                 self._states.pop(printer_id, None)
                 self._state_keys.pop(printer_id, None)
                 self._action_fired.pop(printer_id, None)
+                self._last_class.pop(printer_id, None)
+                self._errors.pop(printer_id, None)
                 continue
 
             await self._check_printer(printer_id, status, settings)
@@ -261,6 +269,17 @@ class ObicoDetectionService:
             timeout=SNAPSHOT_CAPTURE_TIMEOUT,
         )
 
+    def _no_verdict(self, printer_id: int, reason: str) -> None:
+        """Record that this poll produced no verdict for ``printer_id``.
+
+        Kept separate from the classification so the status surface can say
+        "not checking" instead of inheriting the previous verdict — or, worse,
+        the default "safe" a printer used to get before its first inference.
+        """
+        self._errors[printer_id] = reason
+        self._last_error = reason
+        logger.warning(reason)
+
     async def _check_printer(self, printer_id: int, status, settings: dict):
         task_name = getattr(status, "task_name", None) or getattr(status, "subtask_name", "") or ""
         key = f"{task_name}"
@@ -275,17 +294,16 @@ class ObicoDetectionService:
         # keyframe wait.
         frame = await self._capture_frame(printer_id)
         if not frame:
-            self._last_error = f"Failed to capture snapshot for printer {printer_id}"
-            logger.warning(self._last_error)
+            self._no_verdict(printer_id, f"Failed to capture snapshot for printer {printer_id}")
             return
 
         external_url = settings.get("external_url") or ""
         if not external_url:
-            self._last_error = (
+            self._no_verdict(
+                printer_id,
                 "external_url setting is empty — Obico's ML API needs a reachable URL to fetch the snapshot from. "
-                "Set Settings → General → External URL."
+                "Set Settings → General → External URL.",
             )
-            logger.warning(self._last_error)
             return
 
         nonce = await stash_frame(frame)
@@ -304,19 +322,23 @@ class ObicoDetectionService:
                     # Say so plainly: the health endpoint is ungated, so "Test
                     # Connection" passes against exactly this configuration and
                     # a raw 401 gives the user nothing to act on (#2733).
-                    self._last_error = (
+                    #
+                    # Obico's auth decorator runs before the handler, so a call
+                    # rejected here leaves no trace in the ML API's own log —
+                    # which is how #2952 came to be reported as "the loop never
+                    # calls the ML API" while it was calling it every 10s.
+                    self._no_verdict(
+                        printer_id,
                         "Obico ML API rejected the token (401). Set Settings → Failure Detection → "
                         "ML API Token to the ML_API_TOKEN the server runs with, or clear ML_API_TOKEN "
-                        "on the server."
+                        "on the server.",
                     )
-                    logger.warning("%s (printer %s)", self._last_error, printer_id)
                     return
                 resp.raise_for_status()
                 payload = resp.json()
         except Exception as e:
             detail = str(e) or type(e).__name__
-            self._last_error = f"ML API call failed for printer {printer_id}: {detail}"
-            logger.warning(self._last_error)
+            self._no_verdict(printer_id, f"ML API call failed for printer {printer_id}: {detail}")
             return
 
         detections = payload.get("detections", []) if isinstance(payload, dict) else []
@@ -328,6 +350,7 @@ class ObicoDetectionService:
         # A successful capture + ML call clears any transient error from previous
         # polls (typical case: cold-start RTSP timeout on first frame after startup,
         # followed by healthy polls that otherwise leave the banner stuck in the UI).
+        self._errors.pop(printer_id, None)
         self._last_error = None
 
         # Log every non-safe sample — safe samples would flood history
@@ -371,15 +394,39 @@ class ObicoDetectionService:
 
         Only printers with a running, monitored print have a state entry, so
         consumers get "show nothing" for idle printers for free.
+
+        Four classes, and the two non-verdict ones matter as much as the rest:
+
+        ``error``    the most recent poll produced no verdict. ``error`` carries
+                     the reason — a rejected token, an unreachable ML API, a
+                     camera that would not yield a frame, an unset External URL.
+        ``unknown``  monitored, but no inference has come back yet. The state
+                     entry is created when the print is first seen, which is
+                     before the first capture, so this is the honest answer for
+                     that window.
+        ``safe`` / ``warning`` / ``failure``
+                     an actual verdict from an actual inference.
+
+        This used to default to ``safe`` whenever no verdict had been recorded,
+        so a printer whose detection had never once succeeded rendered exactly
+        like a healthy one: a green badge reading "Safe" at score 0.000. That is
+        the worst possible failure mode for a safety feature — it asserts the
+        print is being watched precisely when it is not (#2952).
         """
-        return {
-            pid: {
-                "class": self._last_class.get(pid, "safe"),
+        result = {}
+        for pid, state in self._states.items():
+            error = self._errors.get(pid)
+            if error:
+                verdict = "error"
+            else:
+                verdict = self._last_class.get(pid) or "unknown"
+            result[pid] = {
+                "class": verdict,
                 "frame_count": state.frame_count,
                 "score": round(state.ewm_mean, 4),
+                "error": error,
             }
-            for pid, state in self._states.items()
-        }
+        return result
 
     def get_status(self, sensitivity: str = "medium") -> dict:
         # Report the thresholds for the configured sensitivity, not a hardcoded

backend/tests/integration/test_obico_api.py

@@ -79,10 +79,12 @@ class TestObicoPrinterStatus:
     def clear_detection_state(self):
         obico_detection_service._states.clear()
         obico_detection_service._last_class.clear()
+        obico_detection_service._errors.clear()
         obico_detection_service._last_error = None
         yield
         obico_detection_service._states.clear()
         obico_detection_service._last_class.clear()
+        obico_detection_service._errors.clear()
         obico_detection_service._last_error = None
 
     @pytest.mark.asyncio
@@ -142,3 +144,67 @@ class TestObicoPrinterStatus:
         data = response.json()
         for key in ("ml_url", "action", "history", "poll_interval", "external_url_configured"):
             assert key not in data
+
+
+class TestObicoPrinterStatusNoVerdict:
+    """A printer whose detection is not working must not read as monitored (#2952)."""
+
+    @pytest.fixture(autouse=True)
+    def clear_detection_state(self):
+        obico_detection_service._states.clear()
+        obico_detection_service._last_class.clear()
+        obico_detection_service._errors.clear()
+        obico_detection_service._last_error = None
+        yield
+        obico_detection_service._states.clear()
+        obico_detection_service._last_class.clear()
+        obico_detection_service._errors.clear()
+        obico_detection_service._last_error = None
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_error_class_and_reason_reach_the_card(self, async_client: AsyncClient):
+        obico_detection_service._states[1] = PrintState()
+        obico_detection_service._errors[1] = "Obico ML API rejected the token (401)."
+
+        response = await async_client.get("/api/v1/obico/printer-status")
+        entry = response.json()["per_printer"]["1"]
+        assert entry["class"] == "error"
+        assert entry["error"] == "Obico ML API rejected the token (401)."
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_monitored_but_no_result_yet_is_unknown_not_safe(self, async_client: AsyncClient):
+        obico_detection_service._states[1] = PrintState()
+
+        response = await async_client.get("/api/v1/obico/printer-status")
+        entry = response.json()["per_printer"]["1"]
+        assert entry["class"] == "unknown"
+        assert entry["error"] is None
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_reason_is_withheld_without_settings_read_but_the_class_is_not(self):
+        """The reason can name the ML API base or the External URL, so it stays
+        behind settings:read. Whether the print is being watched is not
+        configuration, so a printers:read user still gets the class."""
+        from unittest.mock import AsyncMock, MagicMock, patch
+
+        from backend.app.api.routes.obico import get_printer_status
+
+        obico_detection_service._states[1] = PrintState()
+        obico_detection_service._errors[1] = "ML API call failed: http://192.168.8.9:3333 refused"
+
+        user = MagicMock()
+        user.has_permission.return_value = False
+
+        # The route calls _load_settings for the enabled/monitored fields; the
+        # redaction under test is independent of them.
+        loaded = {"enabled": True, "enabled_printers": None}
+        with patch.object(obico_detection_service, "_load_settings", new=AsyncMock(return_value=loaded)):
+            data = await get_printer_status(user=user)
+        entry = data["per_printer"][1]
+        assert entry["class"] == "error"
+        assert entry["error"] is None
+        assert data["last_error"] is None
+        assert "192.168.8.9" not in str(data)

backend/tests/unit/test_obico_detection.py

@@ -796,3 +796,156 @@ class TestCheckPrinterUsesCachedFrameUrl:
             await svc._check_printer(1, status, settings)
 
         assert svc._last_error is None
+
+
+class TestNoVerdictIsNotSafe:
+    """A printer nothing is looking at must not report itself as safe (#2952).
+
+    ``get_per_printer`` used to default to ``"safe"`` whenever no verdict had
+    been recorded, and the state entry is created when the print is first seen —
+    before the first capture, let alone the first inference. So a rejected token,
+    an unreachable ML API, a camera that never yields a frame and an unset
+    External URL all rendered as a green "Safe" badge at score 0.000, identical
+    to a healthy monitored print.
+
+    The reporter of #2952 read exactly that, concluded the detection loop had
+    never started, and spent an evening on the network path — while the loop was
+    calling the ML API every 10s and being turned away with a 401 that Obico's
+    auth decorator rejects before its own request log ever sees it.
+    """
+
+    SETTINGS = {
+        "enabled": True,
+        "ml_url": "http://obico:3333",
+        "ml_token": "wrong-token",
+        "sensitivity": "medium",
+        "action": "notify",
+        "poll_interval": 10,
+        "enabled_printers": None,
+        "external_url": "http://bambuddy:8000",
+    }
+
+    @staticmethod
+    def _status():
+        return MagicMock(state="RUNNING", task_name="job", subtask_name="")
+
+    @staticmethod
+    def _client(**kwargs):
+        client = MagicMock()
+        client.get = AsyncMock(**kwargs)
+        client.__aenter__ = AsyncMock(return_value=client)
+        client.__aexit__ = AsyncMock(return_value=False)
+        return client
+
+    @pytest.mark.asyncio
+    async def test_rejected_token_reports_error_and_names_the_setting(self):
+        svc = ObicoDetectionService()
+        response = MagicMock(status_code=401)
+        with (
+            patch(
+                "backend.app.services.obico_detection.httpx.AsyncClient",
+                return_value=self._client(return_value=response),
+            ),
+            patch.object(svc, "_capture_frame", new=AsyncMock(return_value=FAKE_JPEG)),
+        ):
+            await svc._check_printer(1, self._status(), self.SETTINGS)
+
+        entry = svc.get_per_printer()[1]
+        assert entry["class"] == "error"
+        assert "ML API Token" in entry["error"]
+        assert entry["frame_count"] == 0
+
+    @pytest.mark.asyncio
+    async def test_unreachable_ml_api_reports_error(self):
+        svc = ObicoDetectionService()
+        with (
+            patch(
+                "backend.app.services.obico_detection.httpx.AsyncClient",
+                return_value=self._client(side_effect=RuntimeError("connection refused")),
+            ),
+            patch.object(svc, "_capture_frame", new=AsyncMock(return_value=FAKE_JPEG)),
+        ):
+            await svc._check_printer(1, self._status(), self.SETTINGS)
+
+        entry = svc.get_per_printer()[1]
+        assert entry["class"] == "error"
+        assert "connection refused" in entry["error"]
+
+    @pytest.mark.asyncio
+    async def test_failed_capture_reports_error(self):
+        svc = ObicoDetectionService()
+        with patch.object(svc, "_capture_frame", new=AsyncMock(return_value=None)):
+            await svc._check_printer(1, self._status(), self.SETTINGS)
+
+        entry = svc.get_per_printer()[1]
+        assert entry["class"] == "error"
+        assert "capture" in entry["error"].lower()
+
+    @pytest.mark.asyncio
+    async def test_missing_external_url_reports_error(self):
+        svc = ObicoDetectionService()
+        settings = {**self.SETTINGS, "external_url": ""}
+        with patch.object(svc, "_capture_frame", new=AsyncMock(return_value=FAKE_JPEG)):
+            await svc._check_printer(1, self._status(), settings)
+
+        entry = svc.get_per_printer()[1]
+        assert entry["class"] == "error"
+        assert "External URL" in entry["error"]
+
+    @pytest.mark.asyncio
+    async def test_a_recovered_printer_goes_back_to_a_real_verdict(self):
+        """The error must not stick once polling works again — otherwise the
+        badge trades one permanent lie for another."""
+        svc = ObicoDetectionService()
+        with patch.object(svc, "_capture_frame", new=AsyncMock(return_value=None)):
+            await svc._check_printer(1, self._status(), self.SETTINGS)
+        assert svc.get_per_printer()[1]["class"] == "error"
+
+        ok = MagicMock(status_code=200)
+        ok.json.return_value = {"detections": []}
+        ok.raise_for_status = MagicMock()
+        with (
+            patch("backend.app.services.obico_detection.httpx.AsyncClient", return_value=self._client(return_value=ok)),
+            patch.object(svc, "_capture_frame", new=AsyncMock(return_value=FAKE_JPEG)),
+        ):
+            await svc._check_printer(1, self._status(), self.SETTINGS)
+
+        entry = svc.get_per_printer()[1]
+        assert entry["class"] == "safe"
+        assert entry["error"] is None
+        assert entry["frame_count"] == 1
+
+    @pytest.mark.asyncio
+    async def test_state_exists_before_the_first_inference_reports_unknown(self):
+        """The window between "print seen" and "first result" is not safe either."""
+        from backend.app.services.obico_smoothing import PrintState
+
+        svc = ObicoDetectionService()
+        svc._states[1] = PrintState()
+        svc._state_keys[1] = "job"
+
+        entry = svc.get_per_printer()[1]
+        assert entry["class"] == "unknown"
+        assert entry["error"] is None
+
+    @pytest.mark.asyncio
+    async def test_error_is_cleared_when_the_print_ends(self):
+        """A stale error must not carry into the next print's first poll."""
+        svc = ObicoDetectionService()
+        with patch.object(svc, "_capture_frame", new=AsyncMock(return_value=None)):
+            await svc._check_printer(1, self._status(), self.SETTINGS)
+        assert 1 in svc._errors
+
+        idle = MagicMock(state="IDLE", task_name="", subtask_name="")
+        manager = MagicMock()
+        manager.get_all_statuses.return_value = {1: idle}
+        manager.is_connected.return_value = True
+        with patch.dict(
+            "sys.modules",
+            {"backend.app.services.printer_manager": MagicMock(printer_manager=manager)},
+        ):
+            await svc._poll_once(self.SETTINGS)
+
+        assert svc._errors == {}
+        assert svc._last_class == {}
+        assert svc.get_per_printer() == {}