From 51ee5b2c94d01a405041f0f2c1285d879cae7416 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Thu, 18 Jun 2026 13:22:12 -0500 Subject: [PATCH] fix(desktop,tui): surface self-improvement review summary + honor memory_notifications MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "💾 Self-improvement review" summary (skill/memory updated) was invisible on two surfaces: - Desktop Electron app had no review.summary event handler — skill/memory writes happened silently. Now appends a persistent system message to the transcript (matching the Ink TUI's persistent-line semantics, not a transient toast that can be missed). - tui_gateway (backs both 'hermes --tui' and the desktop) never read display.memory_notifications, so it always behaved as 'on' and ignored a user who set 'off'/'verbose'. Added _load_memory_notifications() (mirrors the messaging gateway's bool->str normalization, defaults to 'on') and wired it to agent.memory_notifications, matching gateway/run.py and the CLI. Delivery chain now reaches all surfaces: background_review.py -> background_review_callback -> review.summary event -> desktop transcript / Ink TUI line / gateway message / CLI print. --- .../app/session/hooks/use-message-stream.ts | 27 +++++++++++ .../test_review_summary_callback.py | 45 +++++++++++++++++++ tui_gateway/server.py | 20 +++++++++ 3 files changed, 92 insertions(+) diff --git a/apps/desktop/src/app/session/hooks/use-message-stream.ts b/apps/desktop/src/app/session/hooks/use-message-stream.ts index 3ee52ec8eb7..909c1424796 100644 --- a/apps/desktop/src/app/session/hooks/use-message-stream.ts +++ b/apps/desktop/src/app/session/hooks/use-message-stream.ts @@ -13,6 +13,7 @@ import { type GatewayEventPayload, reasoningPart, renderMediaTags, + textPart, upsertToolPart } from '@/lib/chat-messages' import { coerceGatewayText, coerceThinkingText, normalizePersonalityValue } from '@/lib/chat-runtime' @@ -1080,6 +1081,32 @@ export function useMessageStream({ // completions / watch matches here — re-sync the status stack. void refreshBackgroundProcesses(sessionId) } + } else if (event.type === 'review.summary') { + // Self-improvement background review saved something to memory/skills + // and emitted a persistent summary (Python formats it as + // "💾 Self-improvement review: …"). The CLI prints this via + // prompt_toolkit and the Ink TUI renders it as a system line; the + // desktop has neither, so without this handler the skill/memory + // change happens silently. Surface it as a persistent system message + // in the transcript so the user is always informed — it must not be a + // transient toast that can be missed. + const text = coerceGatewayText(payload?.text).trim() + + if (text && sessionId) { + flushQueuedDeltas(sessionId) + updateSessionState(sessionId, state => ({ + ...state, + messages: [ + ...state.messages, + { + id: `review-summary-${Date.now()}`, + role: 'system', + parts: [textPart(text)], + timestamp: Math.floor(Date.now() / 1000) + } + ] + })) + } } else if (event.type === 'error') { const errorMessage = payload?.message || 'Hermes reported an error' const looksLikeProviderSetup = isProviderSetupErrorMessage(errorMessage) diff --git a/tests/tui_gateway/test_review_summary_callback.py b/tests/tui_gateway/test_review_summary_callback.py index 2c6d3cbeb7c..6ca17889d64 100644 --- a/tests/tui_gateway/test_review_summary_callback.py +++ b/tests/tui_gateway/test_review_summary_callback.py @@ -120,3 +120,48 @@ def test_review_summary_callback_survives_agent_without_attribute(server, monkey # LockedAgent's __slots__ blocks background_review_callback assignment. server._init_session("sid-x", "key-x", LockedAgent(), [], cols=80) # If we got here, _init_session swallowed the AttributeError gracefully. + + +def test_init_session_sets_memory_notifications_from_config(server, monkeypatch): + """_init_session must apply display.memory_notifications to the agent so + the TUI/desktop honors the same off/on/verbose toggle as the messaging + gateway and CLI. Without this the review always behaved as 'on'.""" + monkeypatch.setattr(server, "_SlashWorker", lambda *a, **kw: object()) + monkeypatch.setattr(server, "_wire_callbacks", lambda sid: None) + monkeypatch.setattr(server, "_notify_session_boundary", lambda *a, **kw: None) + monkeypatch.setattr(server, "_session_info", lambda agent, session=None: {"model": "m"}) + monkeypatch.setattr(server, "_load_show_reasoning", lambda: False) + monkeypatch.setattr(server, "_load_tool_progress_mode", lambda: "all") + monkeypatch.setattr(server, "_emit", lambda *a, **kw: None) + monkeypatch.setattr(server, "_load_memory_notifications", lambda: "verbose") + + class FakeAgent: + model = "fake/model" + background_review_callback = None + memory_notifications = "on" + + agent = FakeAgent() + server._init_session("sid-mn", "key-mn", agent, [], cols=80) + + assert agent.memory_notifications == "verbose" + + +@pytest.mark.parametrize( + "raw,expected", + [ + (None, "on"), # unset → default on + ("on", "on"), + ("off", "off"), + ("verbose", "verbose"), + ("VERBOSE", "verbose"), # case-normalized + (True, "on"), # bool back-compat + (False, "off"), + ], +) +def test_load_memory_notifications_normalization(server, monkeypatch, raw, expected): + """_load_memory_notifications mirrors the gateway's bool→str normalization + and defaults to 'on' when the key is absent.""" + display = {} if raw is None else {"memory_notifications": raw} + monkeypatch.setattr(server, "_load_cfg", lambda: {"display": display}) + assert server._load_memory_notifications() == expected + diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 12ee450c6f5..782a8ba457b 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -1907,6 +1907,22 @@ def _load_show_reasoning() -> bool: return bool((_load_cfg().get("display") or {}).get("show_reasoning", False)) +def _load_memory_notifications() -> str: + """Self-improvement review notification mode from config.yaml. + + Parity with the messaging gateway (``gateway/run.py``) and the classic CLI: + ``display.memory_notifications`` controls whether the background review's + "💾 Self-improvement review: …" summary is surfaced. Without this the + TUI/desktop backend always behaved as ``"on"`` and silently ignored a user + who set ``off``. Accepts ``off`` / ``on`` (default) / ``verbose``; a bool is + normalized for back-compat. + """ + raw = (_load_cfg().get("display") or {}).get("memory_notifications") + if isinstance(raw, bool): + return "on" if raw else "off" + return str(raw).lower() if raw else "on" + + def _load_tool_progress_mode() -> str: env = os.environ.get("HERMES_TUI_TOOL_PROGRESS", "").strip().lower() if env in {"off", "new", "all", "verbose"}: @@ -3770,6 +3786,10 @@ def _init_session( agent.background_review_callback = lambda message, _sid=sid: _emit( "review.summary", _sid, {"text": str(message)} ) + # Honor display.memory_notifications (off | on | verbose) like the + # messaging gateway and CLI do — otherwise the review always behaved as + # "on" on the TUI/desktop and a user who set "off" was ignored. + agent.memory_notifications = _load_memory_notifications() except Exception: # Bare AIAgents that don't expose the attribute (unlikely, but keep # session startup resilient).