perf: dedupe lovelace/dashboards/list in ha_config_set_dashboard (#1085)

src/ha_mcp/tools/tools_config_dashboards.py

@@ -265,22 +265,37 @@ def _should_lazy_resolve(error_msg: str) -> bool:
     return _LAZY_RESOLVE_TRIGGER in error_msg
 
 
-async def _resolve_dashboard(client: Any, identifier: str) -> dict[str, str] | None:
+async def _resolve_dashboard(
+    client: Any, identifier: str
+) -> tuple[dict[str, str] | None, list[dict[str, Any]] | None]:
     """Resolve a dashboard identifier (url_path or internal id) to both forms.
 
-    Calls ``lovelace/dashboards/list`` and returns
-    ``{"url_path": ..., "id": ...}`` when the identifier matches either field
-    on a registry entry that has both fields populated; otherwise returns
-    ``None``. Always pays the round-trip when called.
+    Calls ``lovelace/dashboards/list`` and returns a 2-tuple
+    ``(match, dashboards)``:
 
-    Two call sites:
+    - ``match`` is ``{"url_path": ..., "id": ...}`` when the identifier
+      matches either field on a registry entry that has both fields
+      populated; otherwise ``None``.
+    - ``dashboards`` is the raw list as returned by HA when the
+      response shape is recognised (dict-with-``result`` or bare list);
+      ``None`` when the shape was unexpected and a warning was logged.
+
+    Returning ``dashboards`` alongside ``match`` lets callers reuse the
+    list for follow-on checks (existence, id lookup) instead of paying
+    a second ``lovelace/dashboards/list`` round-trip.
+
+    Three call sites:
     - **Lazy fallback** (``_lazy_resolve_and_retry``): only invoked after
       ``lovelace/config`` rejected the identifier with
       ``_LAZY_RESOLVE_TRIGGER`` — the round-trip is gated by the caller.
+      Discards ``dashboards``.
     - **Eager pre-resolve** (``ha_config_set_dashboard``): invoked before
       hyphen validation so callers may pass either form; gated on a
       cheap heuristic ("no hyphen, not 'lovelace'") rather than an error
-      from HA.
+      from HA. Reuses ``dashboards`` for the existence-check below.
+    - **Delete** (``ha_config_delete_dashboard``): resolves either form
+      to the registry id before issuing the delete. Discards
+      ``dashboards``.
     """
     result = await client.send_websocket_message({"type": "lovelace/dashboards/list"})
     if isinstance(result, dict) and "result" in result:
@@ -297,7 +312,7 @@ async def _resolve_dashboard(client: Any, identifier: str) -> dict[str, str] | N
             "treating as no-match",
             type(result).__name__,
         )
-        return None
+        return None, None
 
     for d in dashboards:
         if d.get("id") == identifier or d.get("url_path") == identifier:
@@ -309,8 +324,8 @@ async def _resolve_dashboard(client: Any, identifier: str) -> dict[str, str] | N
                 # would be silently used by callers (e.g.
                 # ``delete_dashboard`` would forward ``resolved_id=""``).
                 continue
-            return {"url_path": url_path, "id": entry_id}
-    return None
+            return {"url_path": url_path, "id": entry_id}, dashboards
+    return None, dashboards
 
 
 @overload
@@ -376,7 +391,7 @@ async def _lazy_resolve_and_retry(
         return url_path, response
 
     try:
-        resolved = await _resolve_dashboard(client, url_path)
+        resolved, _ = await _resolve_dashboard(client, url_path)
     except Exception as resolver_exc:
         # Resolver itself raised (timeout, network blip, etc.). Don't let
         # this exception escape and replace the original HA error with
@@ -941,12 +956,18 @@ async def ha_config_set_dashboard(
             # ``resolved_from`` on the success response so callers can
             # detect this redirect.
             pre_resolved_from: str | None = None
+            # When the pre-resolver fires and finds a match, ``_resolve_dashboard``
+            # has already fetched ``lovelace/dashboards/list``. Capture that list
+            # so the existence-check site below can reuse it instead of paying
+            # a second round-trip.
+            pre_fetched_dashboards: list[dict[str, Any]] | None = None
             if "-" not in url_path and url_path != "lovelace":
-                resolved = await _resolve_dashboard(client, url_path)
+                resolved, dashboards = await _resolve_dashboard(client, url_path)
                 if resolved is not None and resolved["url_path"]:
                     original_url_path = url_path
                     url_path = resolved["url_path"]
                     pre_resolved_from = original_url_path
+                    pre_fetched_dashboards = dashboards
                     logger.info(
                         "ha_config_set_dashboard pre-resolver mapped %r -> %r",
                         original_url_path,
@@ -1129,16 +1150,32 @@ async def ha_config_set_dashboard(
                     transform_result["resolved_from"] = pre_resolved_from
                 return transform_result
 
-            # Check if dashboard exists
-            result = await client.send_websocket_message(
-                {"type": "lovelace/dashboards/list"}
-            )
-            if isinstance(result, dict) and "result" in result:
-                existing_dashboards = result["result"]
-            elif isinstance(result, list):
-                existing_dashboards = result
+            # Check if dashboard exists. When the pre-resolver fired
+            # and matched (internal-id branch), reuse its already-fetched
+            # ``lovelace/dashboards/list`` response to skip a redundant
+            # round-trip — the matched dashboard is guaranteed present in
+            # that list.
+            if pre_fetched_dashboards is not None:
+                existing_dashboards = pre_fetched_dashboards
             else:
-                existing_dashboards = []
+                result = await client.send_websocket_message(
+                    {"type": "lovelace/dashboards/list"}
+                )
+                if isinstance(result, dict) and "result" in result:
+                    existing_dashboards = result["result"]
+                elif isinstance(result, list):
+                    existing_dashboards = result
+                else:
+                    # Mirror the warning emitted by ``_resolve_dashboard`` on
+                    # the same response-shape failure, so a future HA shape
+                    # change shows up at every fetch site rather than going
+                    # silent on this one.
+                    logger.warning(
+                        "lovelace/dashboards/list returned an unexpected shape "
+                        "(type=%s); treating as no-match",
+                        type(result).__name__,
+                    )
+                    existing_dashboards = []
             dashboard_exists = any(
                 d.get("url_path") == url_path for d in existing_dashboards
             )
@@ -1407,7 +1444,7 @@ async def ha_config_delete_dashboard(
         Note: The default dashboard cannot be deleted via this method.
         """
         try:
-            resolved = await _resolve_dashboard(client, url_path)
+            resolved, _ = await _resolve_dashboard(client, url_path)
             if resolved is None:
                 raise_tool_error(
                     create_resource_not_found_error(

tests/src/unit/test_set_dashboard_metadata.py

@@ -21,6 +21,7 @@ def tool_decorator(*args, **kwargs):
             def wrapper(func):
                 self.registered_tools[func.__name__] = func
                 return func
+
             return wrapper
 
         mcp.tool = tool_decorator
@@ -153,3 +154,141 @@ async def test_false_booleans_are_not_filtered_out(self, set_tool, mock_client):
         meta_call = mock_client.send_websocket_message.call_args_list[1][0][0]
         assert meta_call["require_admin"] is False
         assert meta_call["show_in_sidebar"] is False
+
+
+class TestSetDashboardListCallDedup:
+    """When the pre-resolver fires (internal-id branch), the existence-check
+    site reuses the pre-fetched dashboards list rather than issuing a second
+    ``lovelace/dashboards/list`` round-trip.
+
+    The other-branch tests act as regression guards so a future change
+    that re-introduces a redundant list call (or accidentally drops the
+    one fetch on the canonical-url_path branch) is caught here."""
+
+    @pytest.fixture
+    def mock_mcp(self):
+        mcp = MagicMock()
+        self.registered_tools: dict = {}
+
+        def tool_decorator(*args, **kwargs):
+            def wrapper(func):
+                self.registered_tools[func.__name__] = func
+                return func
+
+            return wrapper
+
+        mcp.tool = tool_decorator
+        return mcp
+
+    @pytest.fixture
+    def mock_client(self):
+        client = MagicMock()
+        client.send_websocket_message = AsyncMock()
+        return client
+
+    @pytest.fixture
+    def set_tool(self, mock_mcp, mock_client):
+        from ha_mcp.tools.tools_config_dashboards import register_config_dashboard_tools
+
+        register_config_dashboard_tools(mock_mcp, mock_client)
+        return self.registered_tools["ha_config_set_dashboard"]
+
+    @staticmethod
+    def _list_call_count(mock_client) -> int:
+        return sum(
+            1
+            for c in mock_client.send_websocket_message.call_args_list
+            if c.args and c.args[0].get("type") == "lovelace/dashboards/list"
+        )
+
+    @pytest.mark.asyncio
+    async def test_internal_id_branch_calls_list_only_once(self, set_tool, mock_client):
+        """Pre-resolver fires (hyphenless ``my_dash``) and matches; the
+        existence-check site MUST reuse that list instead of fetching
+        again. Total ``lovelace/dashboards/list`` calls = 1."""
+        dashboards_list = {"result": [{"url_path": "my-dash", "id": "my_dash"}]}
+        mock_client.send_websocket_message.side_effect = [
+            dashboards_list,  # pre-resolver fetch
+            {"success": True},  # metadata update
+        ]
+
+        result = await set_tool(url_path="my_dash", title="Renamed")
+
+        assert self._list_call_count(mock_client) == 1, (
+            "internal-id branch must reuse the pre-resolver's dashboards list"
+        )
+        assert result["success"] is True
+        # Pre-resolver rewrote my_dash -> my-dash; surface marker stays.
+        assert result.get("resolved_from") == "my_dash"
+        # Metadata update did fire on the canonical url_path.
+        meta_call = mock_client.send_websocket_message.call_args_list[1].args[0]
+        assert meta_call["type"] == "lovelace/dashboards/update"
+        assert meta_call["dashboard_id"] == "my_dash"
+
+    @pytest.mark.asyncio
+    async def test_canonical_url_path_branch_still_calls_list_once(
+        self, set_tool, mock_client
+    ):
+        """Already-canonical ``my-dash`` (hyphen present) skips the
+        pre-resolver; the existence-check site still fetches once.
+        Regression guard: total list calls = 1, not 0 (pre-resolver
+        didn't fire) and not 2 (no redundant fetch)."""
+        mock_client.send_websocket_message.side_effect = [
+            {"result": [{"url_path": "my-dash", "id": "my_dash"}]},
+            {"success": True},
+        ]
+
+        result = await set_tool(url_path="my-dash", title="Renamed")
+
+        assert self._list_call_count(mock_client) == 1
+        assert result["success"] is True
+        assert result.get("resolved_from") is None
+
+    @pytest.mark.asyncio
+    async def test_internal_id_no_match_falls_through_to_hyphen_check(
+        self, set_tool, mock_client
+    ):
+        """Hyphenless identifier with no matching dashboard: pre-resolver
+        fetches the list, finds no match, ``url_path`` stays
+        unchanged, then fails the hyphen-validation check before any
+        existence-check fetch can fire. Total list calls = 1."""
+        mock_client.send_websocket_message.side_effect = [
+            {"result": [{"url_path": "other-dash", "id": "other_dash"}]},
+        ]
+
+        with pytest.raises(ToolError) as exc_info:
+            await set_tool(url_path="ghost", title="X")
+
+        body = json.loads(str(exc_info.value))
+        assert "url_path must contain a hyphen" in body["error"]["message"]
+        assert self._list_call_count(mock_client) == 1
+
+    @pytest.mark.asyncio
+    async def test_canonical_url_path_branch_warns_on_unexpected_shape(
+        self, set_tool, mock_client, caplog
+    ):
+        """Existence-check fallback fetch logs a warning on unexpected
+        response shapes, mirroring ``_resolve_dashboard``'s same-arm
+        behaviour. Without this parity, an HA-side shape change would go
+        silent on the canonical-url_path branch — the bug reports would
+        be wedged on ``dashboard_exists = False`` with no operator
+        signal."""
+        import logging
+
+        mock_client.send_websocket_message.side_effect = [
+            "unexpected string",  # response shape failure
+            {"success": True},  # create-dashboard call (still proceeds)
+        ]
+
+        with caplog.at_level(
+            logging.WARNING, logger="ha_mcp.tools.tools_config_dashboards"
+        ):
+            await set_tool(url_path="my-dash", title="New")
+
+        assert any(
+            "unexpected shape" in rec.message and "type=str" in rec.message
+            for rec in caplog.records
+        ), (
+            f"expected an 'unexpected shape' warning naming the response "
+            f"type; got {[rec.message for rec in caplog.records]}"
+        )