diff --git a/nanobot/agent/memory.py b/nanobot/agent/memory.py index 9fbfe0f24..ba964a208 100644 --- a/nanobot/agent/memory.py +++ b/nanobot/agent/memory.py @@ -53,42 +53,6 @@ if TYPE_CHECKING: # --------------------------------------------------------------------------- -class DreamRunProgress: - """Track tool failures that make a nominally completed Dream run unsafe to advance. - - A failure in an earlier tool round is tolerated when the model observed - the error, retried, and the final tool round ran clean. Only failures the - model never got to correct invalidate the run. - """ - - def __init__(self) -> None: - self.had_tool_errors = False - self.last_tool_round_had_errors: bool | None = None - - @property - def recovered_from_tool_errors(self) -> bool: - """True when errors occurred but the final tool round finished clean.""" - return self.had_tool_errors and self.last_tool_round_had_errors is False - - async def __call__( - self, - *_args: Any, - tool_events: list[dict[str, Any]] | None = None, - **_kwargs: Any, - ) -> None: - events = [ - event for event in tool_events or () - if isinstance(cast(object, event), dict) - ] - round_had_errors = any(event.get("phase") == "error" for event in events) - if round_had_errors: - self.had_tool_errors = True - # Terminal payloads ("end"/"error") close a tool round; the most - # recent closed round decides whether earlier failures were recovered. - if any(event.get("phase") in ("end", "error") for event in events): - self.last_tool_round_had_errors = round_had_errors - - class MemoryStore: """Pure file I/O for memory files: MEMORY.md, history.jsonl, SOUL.md, USER.md.""" @@ -704,30 +668,16 @@ class MemoryStore: @staticmethod def dream_run_completed( resp: object | None, - *, - had_tool_errors: bool = False, - recovered_tool_errors: bool = False, ) -> bool: - """Return True only when a Dream turn finished cleanly enough to advance. - - Tool failures from earlier rounds are acceptable when the final tool - round ran clean (``recovered_tool_errors``): the model observed the - failure, corrected it, and produced a consistent final state. Failures - in the final round still invalidate the run. - """ + """Return True when the Dream agent reached a normal terminal response.""" metadata = getattr(resp, "metadata", None) if not isinstance(metadata, dict): return False - if cast(dict[str, Any], metadata).get("_stop_reason") != "completed": - return False - return not had_tool_errors or recovered_tool_errors + return cast(dict[str, Any], metadata).get("_stop_reason") == "completed" @staticmethod def dream_incompletion_reason( resp: object | None, - *, - had_tool_errors: bool = False, - recovered_tool_errors: bool = False, ) -> str: """Human-readable explanation of why a Dream run cannot advance.""" metadata = getattr(resp, "metadata", None) @@ -735,10 +685,7 @@ class MemoryStore: stop_reason = cast(dict[str, Any], metadata).get("_stop_reason", "unknown") else: stop_reason = "missing response metadata" - parts = [f"stop_reason: {stop_reason}"] - if had_tool_errors and not recovered_tool_errors: - parts.append("unrecovered tool errors") - return ", ".join(parts) + return f"stop_reason: {stop_reason}" # -- message formatting utility ------------------------------------------ diff --git a/nanobot/cli/gateway_runtime.py b/nanobot/cli/gateway_runtime.py index aa15b2e64..ad094241b 100644 --- a/nanobot/cli/gateway_runtime.py +++ b/nanobot/cli/gateway_runtime.py @@ -504,13 +504,12 @@ def _run_gateway( # Dream is an internal job — run directly, not through the agent loop. if job.name == "dream": - from nanobot.agent.memory import DreamRunProgress, MemoryStore + from nanobot.agent.memory import MemoryStore dream_session_key = MemoryStore.dream_session_key prune_dream_sessions = MemoryStore.prune_dream_sessions store = agent.context.memory - progress = DreamRunProgress() resp = None diff_body = "" try: @@ -527,17 +526,13 @@ def _run_gateway( session_key=key, ephemeral=True, tools=store.build_dream_tools(), - on_progress=progress, + on_progress=_silent, runtime=dream_runtime, ) - # The real file delta grounds the audit record; clean completion + # The real file delta grounds the audit record; normal completion # decides whether this history batch has finished processing. diff_body = store.dream_content_diff() - completed = MemoryStore.dream_run_completed( - resp, - had_tool_errors=progress.had_tool_errors, - recovered_tool_errors=progress.recovered_from_tool_errors, - ) + completed = MemoryStore.dream_run_completed(resp) if completed: store.set_last_dream_cursor(last_cursor) if diff_body: @@ -554,11 +549,7 @@ def _run_gateway( else: logger.warning( "Dream cron job did not complete ({}); cursor remains at {}", - MemoryStore.dream_incompletion_reason( - resp, - had_tool_errors=progress.had_tool_errors, - recovered_tool_errors=progress.recovered_from_tool_errors, - ), + MemoryStore.dream_incompletion_reason(resp), store.get_last_dream_cursor(), ) except Exception: diff --git a/nanobot/command/builtin.py b/nanobot/command/builtin.py index 15adad011..cff9b76c5 100644 --- a/nanobot/command/builtin.py +++ b/nanobot/command/builtin.py @@ -423,14 +423,16 @@ async def cmd_dream(ctx: CommandContext) -> OutboundMessage: msg = ctx.msg async def _run_dream(): - from nanobot.agent.memory import DreamRunProgress, MemoryStore + from nanobot.agent.memory import MemoryStore + + async def _silent(*_args: Any, **_kwargs: Any) -> None: + pass dream_session_key = MemoryStore.dream_session_key build_dream_commit_message = MemoryStore.build_dream_commit_message prune_dream_sessions = MemoryStore.prune_dream_sessions store = loop.context.memory - progress = DreamRunProgress() content = "" resp = None diff_body = "" @@ -452,18 +454,14 @@ async def cmd_dream(ctx: CommandContext) -> OutboundMessage: session_key=key, ephemeral=True, tools=store.build_dream_tools(), - on_progress=progress, + on_progress=_silent, runtime=dream_runtime, ) elapsed = time.monotonic() - t0 - # The real file delta grounds the audit record; clean completion + # The real file delta grounds the audit record; normal completion # decides whether this history batch has finished processing. diff_body = store.dream_content_diff() - completed = MemoryStore.dream_run_completed( - resp, - had_tool_errors=progress.had_tool_errors, - recovered_tool_errors=progress.recovered_from_tool_errors, - ) + completed = MemoryStore.dream_run_completed(resp) if completed: store.set_last_dream_cursor(last_cursor) if diff_body: @@ -471,11 +469,7 @@ async def cmd_dream(ctx: CommandContext) -> OutboundMessage: else: content = f"Dream completed in {elapsed:.1f}s; no memory changes." else: - reason = MemoryStore.dream_incompletion_reason( - resp, - had_tool_errors=progress.had_tool_errors, - recovered_tool_errors=progress.recovered_from_tool_errors, - ) + reason = MemoryStore.dream_incompletion_reason(resp) content = ( f"Dream did not complete after {elapsed:.1f}s ({reason}); " "memory cursor was not advanced." diff --git a/tests/agent/test_dream.py b/tests/agent/test_dream.py index dcdf21f0a..772898111 100644 --- a/tests/agent/test_dream.py +++ b/tests/agent/test_dream.py @@ -2,7 +2,7 @@ import pytest -from nanobot.agent.memory import DreamRunProgress, MemoryStore +from nanobot.agent.memory import MemoryStore from nanobot.config.schema import ModelPresetConfig from nanobot.providers.base import LLMResponse from nanobot.security.workspace_access import ( @@ -187,95 +187,29 @@ class TestBuildDreamPrompt: class TestDreamRunCompletion: - """DreamRunProgress + dream_run_completed gate cursor advancement.""" + """The runner's terminal state gates Dream cursor advancement.""" class _Resp: def __init__(self, stop_reason: str = "completed") -> None: self.metadata = {"_stop_reason": stop_reason} - @staticmethod - async def _feed(progress: DreamRunProgress, *batches: list[dict]) -> None: - for batch in batches: - await progress("", tool_events=batch) + def test_completed_stop_reason_completes(self): + assert MemoryStore.dream_run_completed(self._Resp()) - @pytest.mark.asyncio - async def test_clean_run_completes(self): - progress = DreamRunProgress() - await self._feed( - progress, - [{"phase": "start"}, {"phase": "end"}], - ) - assert not progress.had_tool_errors - assert not progress.recovered_from_tool_errors - assert MemoryStore.dream_run_completed( - self._Resp(), had_tool_errors=progress.had_tool_errors, - ) + @pytest.mark.parametrize( + "stop_reason", + ["error", "tool_error", "max_iterations", "cancelled"], + ) + def test_non_completed_stop_reason_blocks(self, stop_reason: str): + assert not MemoryStore.dream_run_completed(self._Resp(stop_reason)) - @pytest.mark.asyncio - async def test_recovered_tool_error_completes(self): - """A failed edit_file retry that later succeeds must not block the cursor.""" - progress = DreamRunProgress() - await self._feed( - progress, - [{"phase": "start"}, {"phase": "error"}], - [{"phase": "start"}, {"phase": "end"}], - ) - assert progress.had_tool_errors - assert progress.recovered_from_tool_errors - assert MemoryStore.dream_run_completed( - self._Resp(), - had_tool_errors=progress.had_tool_errors, - recovered_tool_errors=progress.recovered_from_tool_errors, - ) - - @pytest.mark.asyncio - async def test_error_in_final_tool_round_blocks(self): - """An error the model never corrected still invalidates the run.""" - progress = DreamRunProgress() - await self._feed( - progress, - [{"phase": "start"}, {"phase": "end"}], - [{"phase": "start"}, {"phase": "error"}], - ) - assert progress.had_tool_errors - assert not progress.recovered_from_tool_errors - assert not MemoryStore.dream_run_completed( - self._Resp(), - had_tool_errors=progress.had_tool_errors, - recovered_tool_errors=progress.recovered_from_tool_errors, - ) - - @pytest.mark.asyncio - async def test_start_only_events_do_not_close_a_round(self): - progress = DreamRunProgress() - await self._feed(progress, [{"phase": "start"}]) - assert not progress.had_tool_errors - assert progress.last_tool_round_had_errors is None - assert not progress.recovered_from_tool_errors - - @pytest.mark.asyncio - async def test_thought_progress_calls_are_ignored(self): - progress = DreamRunProgress() - await progress("thinking...", tool_hint=True) - await progress("", file_edit_events=[{"phase": "edit"}]) - assert not progress.had_tool_errors - assert progress.last_tool_round_had_errors is None - - def test_non_completed_stop_reason_blocks_despite_clean_tools(self): - assert not MemoryStore.dream_run_completed(self._Resp("max_iterations")) + def test_missing_response_metadata_blocks(self): assert not MemoryStore.dream_run_completed(None) def test_incompletion_reason_names_the_cause(self): - reason = MemoryStore.dream_incompletion_reason( - self._Resp("max_iterations"), - had_tool_errors=True, - recovered_tool_errors=False, - ) - assert "stop_reason: max_iterations" in reason - assert "unrecovered tool errors" in reason assert MemoryStore.dream_incompletion_reason( - self._Resp(), had_tool_errors=True, recovered_tool_errors=True, - ) == "stop_reason: completed" + self._Resp("max_iterations") + ) == "stop_reason: max_iterations" assert MemoryStore.dream_incompletion_reason(None) == ( "stop_reason: missing response metadata" ) @@ -602,6 +536,45 @@ class TestEphemeralDirect: assert resp.metadata["_stop_reason"] == "error" assert MemoryStore.dream_run_completed(resp) is False + async def test_completed_response_after_tool_error_is_success(self, _make_loop): + """A soft tool error is model input, not a second run-level failure state.""" + from unittest.mock import AsyncMock + + from nanobot.providers.base import ToolCallRequest + + loop, store = _make_loop + loop.provider.chat_with_retry = AsyncMock(side_effect=[ + LLMResponse( + content="trying an edit", + finish_reason="tool_calls", + tool_calls=[ToolCallRequest( + id="call_edit", + name="edit_file", + arguments={ + "path": "SOUL.md", + "old_text": "text that is not present", + "new_text": "replacement", + }, + )], + usage={}, + ), + LLMResponse(content="done", finish_reason="stop", tool_calls=[], usage={}), + ]) + + resp = await loop.process_direct( + "test", + session_key="dream:handled-tool-error", + ephemeral=True, + tools=store.build_dream_tools(), + ) + + assert resp is not None + assert resp.metadata["_stop_reason"] == "completed" + assert MemoryStore.dream_run_completed(resp) is True + second_request = loop.provider.chat_with_retry.await_args_list[1].kwargs["messages"] + tool_result = next(message for message in second_request if message["role"] == "tool") + assert "Error" in tool_result["content"] + async def test_dream_turn_can_skip_unbatched_recent_history(self, tmp_path): """Dream must only see the batch selected by build_dream_prompt.""" from unittest.mock import MagicMock diff --git a/tests/command/test_builtin_dream.py b/tests/command/test_builtin_dream.py index 7b1f49931..bdcb8168a 100644 --- a/tests/command/test_builtin_dream.py +++ b/tests/command/test_builtin_dream.py @@ -274,8 +274,8 @@ async def test_dream_keeps_cursor_when_incomplete_with_diff(tmp_path) -> None: @pytest.mark.asyncio -async def test_dream_keeps_cursor_when_completed_after_tool_error(tmp_path) -> None: - """A soft tool failure must not masquerade as a verified no-op.""" +async def test_dream_advances_cursor_when_completed_after_tool_error(tmp_path) -> None: + """A handled tool failure does not invalidate a normally completed run.""" ctx, store = _build_runnable_dream( tmp_path, initialized=True, @@ -284,8 +284,8 @@ async def test_dream_keeps_cursor_when_completed_after_tool_error(tmp_path) -> N ) await cmd_dream(ctx) await asyncio.sleep(0) - assert store._last_dream_cursor == 5 - assert "did not complete" in ctx.loop.bus.outbound[0].content + assert store._last_dream_cursor == 42 + assert "no memory changes" in ctx.loop.bus.outbound[0].content @pytest.mark.asyncio @@ -339,7 +339,7 @@ async def test_dream_noop_batch_unlocks_following_history(tmp_path) -> None: @pytest.mark.asyncio async def test_dream_non_git_falls_back_to_completion_gate(tmp_path) -> None: - """Non-git workspaces use the same clean-completion gate.""" + """Non-git workspaces use the same normal-completion gate.""" ctx, store = _build_runnable_dream( tmp_path, initialized=False, content_diff="", stop_reason="completed", )