fix(cli): surface self-improvement review summaries from bg thread
When the self-improvement background review fires after a turn, it runs in a bg thread and emits a ' 💾 <summary>' line to announce what it saved to memory or skills. Two problems made this invisible to users even when the review successfully modified a skill: 1. The print went through `_cprint` (prompt_toolkit's print_formatted_text) on a bg thread while the CLI's PromptSession was live. Direct print_formatted_text races with the input-area redraw and the line can land behind/above the prompt, scrolled off without the user seeing it. 2. The message said only '💾 Skill created.' / '💾 Memory updated' with no indication that the self-improvement loop was the one doing this. Users who did catch the line couldn't tell the background review from some other agent action. Fixes: - `_cprint` now detects when it's called from a non-app thread with a running prompt_toolkit Application, and routes through `run_in_terminal` via `loop.call_soon_threadsafe`. That pauses the input, prints the line above the prompt, and redraws — the normal prompt_toolkit contract for bg-thread output. Direct-print fallback preserved for the no-app / same-thread / import-error paths. Affects every bg-thread emission, not just the review summary (curator summaries and auxiliary failure prints benefit too). - The summary now reads ' 💾 Self-improvement review: <summary>' in both the CLI and the gateway `background_review_callback` path, so the origin is unambiguous. Tests: - New `tests/cli/test_cprint_bg_thread.py` covers all five routing branches (no app, app-not-running, cross-thread schedule, same-thread direct, app-loop-attribute-error, import-error). - New case in `tests/run_agent/test_background_review.py` asserts the attributed prefix shows up in both `_safe_print` and `background_review_callback`. Live E2E: exercised _cprint from a bg thread inside a real Application event loop; confirmed get_app_or_none() sees the app, call_soon_threadsafe schedules run_in_terminal, and the inner _pt_print runs.
This commit is contained in:
@@ -127,3 +127,66 @@ def test_background_review_installs_auto_deny_approval_callback(monkeypatch):
|
||||
"Background review leaked its approval callback into the worker "
|
||||
"thread's TLS slot; a recycled thread-id could reuse it."
|
||||
)
|
||||
|
||||
|
||||
def test_background_review_summary_is_attributed_to_self_improvement_loop(monkeypatch):
|
||||
"""The CLI/gateway emission must identify the self-improvement loop.
|
||||
|
||||
Users who miss the line in their terminal have no way to tell that the
|
||||
background review was what modified their skill/memory stores. The
|
||||
summary prefix ``💾 Self-improvement review: …`` makes the origin
|
||||
explicit so both the CLI and gateway deliveries are unambiguous.
|
||||
"""
|
||||
import json
|
||||
|
||||
captured_prints: list = []
|
||||
captured_bg_callback: list = []
|
||||
|
||||
class FakeReviewAgent:
|
||||
def __init__(self, **kwargs):
|
||||
# Simulate a review that successfully updated memory so
|
||||
# _summarize_background_review_actions returns a real action.
|
||||
self._session_messages = [
|
||||
{
|
||||
"role": "tool",
|
||||
"tool_call_id": "call_bg",
|
||||
"content": json.dumps(
|
||||
{"success": True, "message": "Entry added", "target": "memory"}
|
||||
),
|
||||
}
|
||||
]
|
||||
|
||||
def run_conversation(self, **kwargs):
|
||||
pass
|
||||
|
||||
def shutdown_memory_provider(self):
|
||||
pass
|
||||
|
||||
def close(self):
|
||||
pass
|
||||
|
||||
monkeypatch.setattr(run_agent_module, "AIAgent", FakeReviewAgent)
|
||||
monkeypatch.setattr(run_agent_module.threading, "Thread", ImmediateThread)
|
||||
|
||||
agent = _bare_agent()
|
||||
agent._safe_print = lambda *a, **kw: captured_prints.append(" ".join(str(x) for x in a))
|
||||
agent.background_review_callback = lambda msg: captured_bg_callback.append(msg)
|
||||
|
||||
AIAgent._spawn_background_review(
|
||||
agent,
|
||||
messages_snapshot=[{"role": "user", "content": "hi"}],
|
||||
review_memory=True,
|
||||
)
|
||||
|
||||
# Exactly one summary should have been emitted, and it must identify
|
||||
# the self-improvement review explicitly.
|
||||
assert len(captured_prints) == 1, captured_prints
|
||||
printed = captured_prints[0]
|
||||
assert "Self-improvement review" in printed, printed
|
||||
assert "Memory updated" in printed, printed
|
||||
|
||||
# Gateway path gets the same prefix.
|
||||
assert len(captured_bg_callback) == 1
|
||||
assert captured_bg_callback[0].startswith("💾 Self-improvement review:"), (
|
||||
captured_bg_callback[0]
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user