[loop-cycle-392] refactor: DRY broadcast + bark error logging (#397, #398) (#400)

src/dashboard/routes/world.py

@@ -151,14 +151,8 @@ async def world_ws(websocket: WebSocket) -> None:
         logger.info("World WS disconnected — %d clients", len(_ws_clients))
 
 
-async def broadcast_world_state(presence: dict) -> None:
-    """Broadcast a ``timmy_state`` message to all connected Workshop clients.
-
-    Called by :class:`~timmy.workshop_state.WorkshopHeartbeat` via its
-    ``on_change`` callback.
-    """
-    state = _build_world_state(presence)
-    message = json.dumps({"type": "timmy_state", **state["timmyState"]})
+async def _broadcast(message: str) -> None:
+    """Send *message* to every connected Workshop client, pruning dead ones."""
     dead: list[WebSocket] = []
     for ws in _ws_clients:
         try:
@@ -170,6 +164,16 @@ async def broadcast_world_state(presence: dict) -> None:
             _ws_clients.remove(ws)
 
 
+async def broadcast_world_state(presence: dict) -> None:
+    """Broadcast a ``timmy_state`` message to all connected Workshop clients.
+
+    Called by :class:`~timmy.workshop_state.WorkshopHeartbeat` via its
+    ``on_change`` callback.
+    """
+    state = _build_world_state(presence)
+    await _broadcast(json.dumps({"type": "timmy_state", **state["timmyState"]}))
+
+
 # ---------------------------------------------------------------------------
 # Visitor chat — bark engine
 # ---------------------------------------------------------------------------
@@ -185,26 +189,35 @@ async def _handle_client_message(raw: str) -> None:
     if data.get("type") == "visitor_message":
         text = (data.get("text") or "").strip()
         if text:
-            asyncio.create_task(_bark_and_broadcast(text))
+            task = asyncio.create_task(_bark_and_broadcast(text))
+            task.add_done_callback(_log_bark_failure)
+
+
+def _log_bark_failure(task: asyncio.Task) -> None:
+    """Log unhandled exceptions from fire-and-forget bark tasks."""
+    if task.cancelled():
+        return
+    exc = task.exception()
+    if exc is not None:
+        logger.error("Bark task failed: %s", exc)
 
 
 async def _bark_and_broadcast(visitor_text: str) -> None:
     """Generate a bark response and broadcast it to all Workshop clients."""
-    # Signal "thinking" state
-    await _broadcast_speech({"type": "timmy_thinking"})
+    await _broadcast(json.dumps({"type": "timmy_thinking"}))
 
     reply = await _generate_bark(visitor_text)
 
-    # Store exchange in conversation buffer
     _conversation.append({"visitor": visitor_text, "timmy": reply})
 
-    # Broadcast speech bubble + conversation history
-    await _broadcast_speech(
-        {
-            "type": "timmy_speech",
-            "text": reply,
-            "recentExchanges": list(_conversation),
-        }
+    await _broadcast(
+        json.dumps(
+            {
+                "type": "timmy_speech",
+                "text": reply,
+                "recentExchanges": list(_conversation),
+            }
+        )
     )
 
 
@@ -222,17 +235,3 @@ async def _generate_bark(visitor_text: str) -> str:
     except Exception as exc:
         logger.warning("Bark generation failed: %s", exc)
         return "Hmm, my thoughts are a bit tangled right now."
-
-
-async def _broadcast_speech(payload: dict) -> None:
-    """Broadcast a speech message to all connected Workshop clients."""
-    message = json.dumps(payload)
-    dead: list[WebSocket] = []
-    for ws in _ws_clients:
-        try:
-            await ws.send_text(message)
-        except Exception:
-            dead.append(ws)
-    for ws in dead:
-        if ws in _ws_clients:
-            _ws_clients.remove(ws)

tests/dashboard/test_world_api.py

@@ -1,19 +1,21 @@
 """Tests for GET /api/world/state endpoint and /api/world/ws relay."""
 
 import json
+import logging
 import time
-from unittest.mock import AsyncMock, patch
+from unittest.mock import AsyncMock, MagicMock, patch
 
 import pytest
 
 from dashboard.routes.world import (
     _STALE_THRESHOLD,
     _bark_and_broadcast,
-    _broadcast_speech,
+    _broadcast,
     _build_world_state,
     _conversation,
     _generate_bark,
     _handle_client_message,
+    _log_bark_failure,
     _read_presence_file,
     broadcast_world_state,
 )
@@ -331,15 +333,15 @@ async def test_bark_and_broadcast_sends_thinking_then_speech():
 
 
 @pytest.mark.asyncio
-async def test_broadcast_speech_removes_dead_clients():
-    """Dead clients are cleaned up during speech broadcast."""
+async def test_broadcast_removes_dead_clients():
+    """Dead clients are cleaned up during broadcast."""
     from dashboard.routes.world import _ws_clients
 
     dead = AsyncMock()
     dead.send_text.side_effect = ConnectionError("gone")
     _ws_clients.append(dead)
     try:
-        await _broadcast_speech({"type": "timmy_speech", "text": "test"})
+        await _broadcast(json.dumps({"type": "timmy_speech", "text": "test"}))
         assert dead not in _ws_clients
     finally:
         _ws_clients.clear()
@@ -368,3 +370,29 @@ async def test_conversation_buffer_caps_at_max():
     finally:
         _ws_clients.clear()
         _conversation.clear()
+
+
+def test_log_bark_failure_logs_exception(caplog):
+    """_log_bark_failure logs errors from failed bark tasks."""
+    import asyncio
+
+    loop = asyncio.new_event_loop()
+
+    async def _fail():
+        raise RuntimeError("bark boom")
+
+    task = loop.create_task(_fail())
+    loop.run_until_complete(asyncio.sleep(0.01))
+    loop.close()
+    with caplog.at_level(logging.ERROR):
+        _log_bark_failure(task)
+    assert "bark boom" in caplog.text
+
+
+def test_log_bark_failure_ignores_cancelled():
+    """_log_bark_failure silently ignores cancelled tasks."""
+    import asyncio
+
+    task = MagicMock(spec=asyncio.Task)
+    task.cancelled.return_value = True
+    _log_bark_failure(task)  # should not raise