fix(desktop,tui): surface self-improvement review summary + honor memory_notifications
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.
This commit is contained in:
parent
07e785d60a
commit
51ee5b2c94
@ -13,6 +13,7 @@ import {
|
|||||||
type GatewayEventPayload,
|
type GatewayEventPayload,
|
||||||
reasoningPart,
|
reasoningPart,
|
||||||
renderMediaTags,
|
renderMediaTags,
|
||||||
|
textPart,
|
||||||
upsertToolPart
|
upsertToolPart
|
||||||
} from '@/lib/chat-messages'
|
} from '@/lib/chat-messages'
|
||||||
import { coerceGatewayText, coerceThinkingText, normalizePersonalityValue } from '@/lib/chat-runtime'
|
import { coerceGatewayText, coerceThinkingText, normalizePersonalityValue } from '@/lib/chat-runtime'
|
||||||
@ -1080,6 +1081,32 @@ export function useMessageStream({
|
|||||||
// completions / watch matches here — re-sync the status stack.
|
// completions / watch matches here — re-sync the status stack.
|
||||||
void refreshBackgroundProcesses(sessionId)
|
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') {
|
} else if (event.type === 'error') {
|
||||||
const errorMessage = payload?.message || 'Hermes reported an error'
|
const errorMessage = payload?.message || 'Hermes reported an error'
|
||||||
const looksLikeProviderSetup = isProviderSetupErrorMessage(errorMessage)
|
const looksLikeProviderSetup = isProviderSetupErrorMessage(errorMessage)
|
||||||
|
|||||||
@ -120,3 +120,48 @@ def test_review_summary_callback_survives_agent_without_attribute(server, monkey
|
|||||||
# LockedAgent's __slots__ blocks background_review_callback assignment.
|
# LockedAgent's __slots__ blocks background_review_callback assignment.
|
||||||
server._init_session("sid-x", "key-x", LockedAgent(), [], cols=80)
|
server._init_session("sid-x", "key-x", LockedAgent(), [], cols=80)
|
||||||
# If we got here, _init_session swallowed the AttributeError gracefully.
|
# 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
|
||||||
|
|
||||||
|
|||||||
@ -1907,6 +1907,22 @@ def _load_show_reasoning() -> bool:
|
|||||||
return bool((_load_cfg().get("display") or {}).get("show_reasoning", False))
|
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:
|
def _load_tool_progress_mode() -> str:
|
||||||
env = os.environ.get("HERMES_TUI_TOOL_PROGRESS", "").strip().lower()
|
env = os.environ.get("HERMES_TUI_TOOL_PROGRESS", "").strip().lower()
|
||||||
if env in {"off", "new", "all", "verbose"}:
|
if env in {"off", "new", "all", "verbose"}:
|
||||||
@ -3770,6 +3786,10 @@ def _init_session(
|
|||||||
agent.background_review_callback = lambda message, _sid=sid: _emit(
|
agent.background_review_callback = lambda message, _sid=sid: _emit(
|
||||||
"review.summary", _sid, {"text": str(message)}
|
"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:
|
except Exception:
|
||||||
# Bare AIAgents that don't expose the attribute (unlikely, but keep
|
# Bare AIAgents that don't expose the attribute (unlikely, but keep
|
||||||
# session startup resilient).
|
# session startup resilient).
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user