From 66f728e7a37670ef733b36dc45f3a8334cb31da8 Mon Sep 17 00:00:00 2001 From: Ed_ Date: Mon, 23 Feb 2026 16:45:34 -0500 Subject: [PATCH] fix(conductor): Apply review suggestions for track 'event_driven_metrics_20260223' --- ai_client.py | 10 +++++++++- events.py | 29 ++++++++++++++++++++++++++--- gui.py | 33 +++++++++++++++++---------------- tests/test_gui_updates.py | 6 +++--- 4 files changed, 55 insertions(+), 23 deletions(-) diff --git a/ai_client.py b/ai_client.py index 711225a..cd6d8f4 100644 --- a/ai_client.py +++ b/ai_client.py @@ -632,7 +632,15 @@ def _send_gemini(md_content: str, user_message: str, base_dir: str, file_items: if cached_tokens: usage["cache_read_input_tokens"] = cached_tokens - events.emit("response_received", payload={"provider": "gemini", "model": _model, "usage": usage, "round": r_idx}) + # Fetch cache stats in the background thread to avoid blocking GUI + cache_stats = None + try: + cache_stats = get_gemini_cache_stats() + except Exception: + pass + + events.emit("response_received", payload={"provider": "gemini", "model": _model, "usage": usage, "round": r_idx, "cache_stats": cache_stats}) + reason = resp.candidates[0].finish_reason.name if resp.candidates and hasattr(resp.candidates[0], "finish_reason") else "STOP" _append_comms("IN", "response", {"round": r_idx, "stop_reason": reason, "text": txt, "tool_calls": [{"name": c.name, "args": dict(c.args)} for c in calls], "usage": usage}) diff --git a/events.py b/events.py index 3368692..8cc59c9 100644 --- a/events.py +++ b/events.py @@ -1,14 +1,37 @@ +""" +Decoupled event emission system for cross-module communication. +""" +from typing import Callable, Any, Dict, List class EventEmitter: + """ + Simple event emitter for decoupled communication between modules. + """ def __init__(self): - self._listeners = {} + """Initializes the EventEmitter with an empty listener map.""" + self._listeners: Dict[str, List[Callable]] = {} - def on(self, event_name, callback): + def on(self, event_name: str, callback: Callable): + """ + Registers a callback for a specific event. + + Args: + event_name: The name of the event to listen for. + callback: The function to call when the event is emitted. + """ if event_name not in self._listeners: self._listeners[event_name] = [] self._listeners[event_name].append(callback) - def emit(self, event_name, *args, **kwargs): + def emit(self, event_name: str, *args: Any, **kwargs: Any): + """ + Emits an event, calling all registered callbacks. + + Args: + event_name: The name of the event to emit. + *args: Positional arguments to pass to callbacks. + **kwargs: Keyword arguments to pass to callbacks. + """ if event_name in self._listeners: for callback in self._listeners[event_name]: callback(*args, **kwargs) diff --git a/gui.py b/gui.py index f66521f..af32207 100644 --- a/gui.py +++ b/gui.py @@ -829,11 +829,13 @@ class App: def _on_api_event(self, *args, **kwargs): """Callback for ai_client events. Queues a telemetry refresh on the main thread.""" + payload = kwargs.get("payload", {}) with self._pending_gui_tasks_lock: - self._pending_gui_tasks.append({"action": "refresh_api_metrics"}) + self._pending_gui_tasks.append({"action": "refresh_api_metrics", "payload": payload}) - def _refresh_api_metrics(self): + def _refresh_api_metrics(self, payload: dict = None): """Updates the token budget and cache stats visualizers.""" + payload = payload or {} self._last_bleed_update_time = time.time() # History bleed @@ -846,21 +848,20 @@ class App: limit = stats.get("limit", 0) dpg.set_value("token_budget_label", f"{current:,} / {limit:,}") - # Gemini cache + # Gemini cache - Use payload data to avoid blocking the main thread with network calls if dpg.does_item_exist("gemini_cache_label"): - if self.current_provider == "gemini": - try: - cache_stats = ai_client.get_gemini_cache_stats() - count = cache_stats.get("cache_count", 0) - size_bytes = cache_stats.get("total_size_bytes", 0) - size_kb = size_bytes / 1024.0 - text = f"Gemini Caches: {count} ({size_kb:.1f} KB)" - dpg.set_value("gemini_cache_label", text) - dpg.configure_item("gemini_cache_label", show=True) - except Exception: - dpg.configure_item("gemini_cache_label", show=False) - else: + cache_stats = payload.get("cache_stats") + if cache_stats: + count = cache_stats.get("cache_count", 0) + size_bytes = cache_stats.get("total_size_bytes", 0) + size_kb = size_bytes / 1024.0 + text = f"Gemini Caches: {count} ({size_kb:.1f} KB)" + dpg.set_value("gemini_cache_label", text) + dpg.configure_item("gemini_cache_label", show=True) + elif self.current_provider != "gemini": dpg.configure_item("gemini_cache_label", show=False) + # Note: We don't hide it if no stats are in payload, + # to avoid flickering during tool/chunk events that don't include stats. def _update_performance_diagnostics(self): """Updates performance diagnostics displays (throttled).""" @@ -2254,7 +2255,7 @@ class App: if cb: cb() elif action == "refresh_api_metrics": - self._refresh_api_metrics() + self._refresh_api_metrics(task.get("payload", {})) except Exception as e: print(f"Error executing GUI hook task: {e}") diff --git a/tests/test_gui_updates.py b/tests/test_gui_updates.py index 2834a39..e39bfae 100644 --- a/tests/test_gui_updates.py +++ b/tests/test_gui_updates.py @@ -103,11 +103,11 @@ def test_cache_data_display_updates_correctly(app_instance): # We also need to mock get_history_bleed_stats as it's called in the same function with patch('ai_client.get_history_bleed_stats', return_value={}): - # 4. Call the method under test - app_instance._refresh_api_metrics() + # 4. Call the method under test with payload + app_instance._refresh_api_metrics(payload={'cache_stats': mock_cache_stats}) # 5. Assert the results - mock_get_cache_stats.assert_called_once() + # mock_get_cache_stats.assert_called_once() # No longer called synchronously # Check that the UI item was shown and its value was set mock_configure_item.assert_any_call("gemini_cache_label", show=True)