mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-09-01 00:31:51 +03:00
fix(dream): gate cursor on run completion
This commit is contained in:
+3
-56
@@ -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:
|
class MemoryStore:
|
||||||
"""Pure file I/O for memory files: MEMORY.md, history.jsonl, SOUL.md, USER.md."""
|
"""Pure file I/O for memory files: MEMORY.md, history.jsonl, SOUL.md, USER.md."""
|
||||||
|
|
||||||
@@ -704,30 +668,16 @@ class MemoryStore:
|
|||||||
@staticmethod
|
@staticmethod
|
||||||
def dream_run_completed(
|
def dream_run_completed(
|
||||||
resp: object | None,
|
resp: object | None,
|
||||||
*,
|
|
||||||
had_tool_errors: bool = False,
|
|
||||||
recovered_tool_errors: bool = False,
|
|
||||||
) -> bool:
|
) -> bool:
|
||||||
"""Return True only when a Dream turn finished cleanly enough to advance.
|
"""Return True when the Dream agent reached a normal terminal response."""
|
||||||
|
|
||||||
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.
|
|
||||||
"""
|
|
||||||
metadata = getattr(resp, "metadata", None)
|
metadata = getattr(resp, "metadata", None)
|
||||||
if not isinstance(metadata, dict):
|
if not isinstance(metadata, dict):
|
||||||
return False
|
return False
|
||||||
if cast(dict[str, Any], metadata).get("_stop_reason") != "completed":
|
return cast(dict[str, Any], metadata).get("_stop_reason") == "completed"
|
||||||
return False
|
|
||||||
return not had_tool_errors or recovered_tool_errors
|
|
||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
def dream_incompletion_reason(
|
def dream_incompletion_reason(
|
||||||
resp: object | None,
|
resp: object | None,
|
||||||
*,
|
|
||||||
had_tool_errors: bool = False,
|
|
||||||
recovered_tool_errors: bool = False,
|
|
||||||
) -> str:
|
) -> str:
|
||||||
"""Human-readable explanation of why a Dream run cannot advance."""
|
"""Human-readable explanation of why a Dream run cannot advance."""
|
||||||
metadata = getattr(resp, "metadata", None)
|
metadata = getattr(resp, "metadata", None)
|
||||||
@@ -735,10 +685,7 @@ class MemoryStore:
|
|||||||
stop_reason = cast(dict[str, Any], metadata).get("_stop_reason", "unknown")
|
stop_reason = cast(dict[str, Any], metadata).get("_stop_reason", "unknown")
|
||||||
else:
|
else:
|
||||||
stop_reason = "missing response metadata"
|
stop_reason = "missing response metadata"
|
||||||
parts = [f"stop_reason: {stop_reason}"]
|
return f"stop_reason: {stop_reason}"
|
||||||
if had_tool_errors and not recovered_tool_errors:
|
|
||||||
parts.append("unrecovered tool errors")
|
|
||||||
return ", ".join(parts)
|
|
||||||
|
|
||||||
# -- message formatting utility ------------------------------------------
|
# -- message formatting utility ------------------------------------------
|
||||||
|
|
||||||
|
|||||||
@@ -504,13 +504,12 @@ def _run_gateway(
|
|||||||
|
|
||||||
# Dream is an internal job — run directly, not through the agent loop.
|
# Dream is an internal job — run directly, not through the agent loop.
|
||||||
if job.name == "dream":
|
if job.name == "dream":
|
||||||
from nanobot.agent.memory import DreamRunProgress, MemoryStore
|
from nanobot.agent.memory import MemoryStore
|
||||||
|
|
||||||
dream_session_key = MemoryStore.dream_session_key
|
dream_session_key = MemoryStore.dream_session_key
|
||||||
prune_dream_sessions = MemoryStore.prune_dream_sessions
|
prune_dream_sessions = MemoryStore.prune_dream_sessions
|
||||||
|
|
||||||
store = agent.context.memory
|
store = agent.context.memory
|
||||||
progress = DreamRunProgress()
|
|
||||||
resp = None
|
resp = None
|
||||||
diff_body = ""
|
diff_body = ""
|
||||||
try:
|
try:
|
||||||
@@ -527,17 +526,13 @@ def _run_gateway(
|
|||||||
session_key=key,
|
session_key=key,
|
||||||
ephemeral=True,
|
ephemeral=True,
|
||||||
tools=store.build_dream_tools(),
|
tools=store.build_dream_tools(),
|
||||||
on_progress=progress,
|
on_progress=_silent,
|
||||||
runtime=dream_runtime,
|
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.
|
# decides whether this history batch has finished processing.
|
||||||
diff_body = store.dream_content_diff()
|
diff_body = store.dream_content_diff()
|
||||||
completed = MemoryStore.dream_run_completed(
|
completed = MemoryStore.dream_run_completed(resp)
|
||||||
resp,
|
|
||||||
had_tool_errors=progress.had_tool_errors,
|
|
||||||
recovered_tool_errors=progress.recovered_from_tool_errors,
|
|
||||||
)
|
|
||||||
if completed:
|
if completed:
|
||||||
store.set_last_dream_cursor(last_cursor)
|
store.set_last_dream_cursor(last_cursor)
|
||||||
if diff_body:
|
if diff_body:
|
||||||
@@ -554,11 +549,7 @@ def _run_gateway(
|
|||||||
else:
|
else:
|
||||||
logger.warning(
|
logger.warning(
|
||||||
"Dream cron job did not complete ({}); cursor remains at {}",
|
"Dream cron job did not complete ({}); cursor remains at {}",
|
||||||
MemoryStore.dream_incompletion_reason(
|
MemoryStore.dream_incompletion_reason(resp),
|
||||||
resp,
|
|
||||||
had_tool_errors=progress.had_tool_errors,
|
|
||||||
recovered_tool_errors=progress.recovered_from_tool_errors,
|
|
||||||
),
|
|
||||||
store.get_last_dream_cursor(),
|
store.get_last_dream_cursor(),
|
||||||
)
|
)
|
||||||
except Exception:
|
except Exception:
|
||||||
|
|||||||
@@ -423,14 +423,16 @@ async def cmd_dream(ctx: CommandContext) -> OutboundMessage:
|
|||||||
msg = ctx.msg
|
msg = ctx.msg
|
||||||
|
|
||||||
async def _run_dream():
|
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
|
dream_session_key = MemoryStore.dream_session_key
|
||||||
build_dream_commit_message = MemoryStore.build_dream_commit_message
|
build_dream_commit_message = MemoryStore.build_dream_commit_message
|
||||||
prune_dream_sessions = MemoryStore.prune_dream_sessions
|
prune_dream_sessions = MemoryStore.prune_dream_sessions
|
||||||
|
|
||||||
store = loop.context.memory
|
store = loop.context.memory
|
||||||
progress = DreamRunProgress()
|
|
||||||
content = ""
|
content = ""
|
||||||
resp = None
|
resp = None
|
||||||
diff_body = ""
|
diff_body = ""
|
||||||
@@ -452,18 +454,14 @@ async def cmd_dream(ctx: CommandContext) -> OutboundMessage:
|
|||||||
session_key=key,
|
session_key=key,
|
||||||
ephemeral=True,
|
ephemeral=True,
|
||||||
tools=store.build_dream_tools(),
|
tools=store.build_dream_tools(),
|
||||||
on_progress=progress,
|
on_progress=_silent,
|
||||||
runtime=dream_runtime,
|
runtime=dream_runtime,
|
||||||
)
|
)
|
||||||
elapsed = time.monotonic() - t0
|
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.
|
# decides whether this history batch has finished processing.
|
||||||
diff_body = store.dream_content_diff()
|
diff_body = store.dream_content_diff()
|
||||||
completed = MemoryStore.dream_run_completed(
|
completed = MemoryStore.dream_run_completed(resp)
|
||||||
resp,
|
|
||||||
had_tool_errors=progress.had_tool_errors,
|
|
||||||
recovered_tool_errors=progress.recovered_from_tool_errors,
|
|
||||||
)
|
|
||||||
if completed:
|
if completed:
|
||||||
store.set_last_dream_cursor(last_cursor)
|
store.set_last_dream_cursor(last_cursor)
|
||||||
if diff_body:
|
if diff_body:
|
||||||
@@ -471,11 +469,7 @@ async def cmd_dream(ctx: CommandContext) -> OutboundMessage:
|
|||||||
else:
|
else:
|
||||||
content = f"Dream completed in {elapsed:.1f}s; no memory changes."
|
content = f"Dream completed in {elapsed:.1f}s; no memory changes."
|
||||||
else:
|
else:
|
||||||
reason = MemoryStore.dream_incompletion_reason(
|
reason = MemoryStore.dream_incompletion_reason(resp)
|
||||||
resp,
|
|
||||||
had_tool_errors=progress.had_tool_errors,
|
|
||||||
recovered_tool_errors=progress.recovered_from_tool_errors,
|
|
||||||
)
|
|
||||||
content = (
|
content = (
|
||||||
f"Dream did not complete after {elapsed:.1f}s ({reason}); "
|
f"Dream did not complete after {elapsed:.1f}s ({reason}); "
|
||||||
"memory cursor was not advanced."
|
"memory cursor was not advanced."
|
||||||
|
|||||||
+52
-79
@@ -2,7 +2,7 @@
|
|||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
from nanobot.agent.memory import DreamRunProgress, MemoryStore
|
from nanobot.agent.memory import MemoryStore
|
||||||
from nanobot.config.schema import ModelPresetConfig
|
from nanobot.config.schema import ModelPresetConfig
|
||||||
from nanobot.providers.base import LLMResponse
|
from nanobot.providers.base import LLMResponse
|
||||||
from nanobot.security.workspace_access import (
|
from nanobot.security.workspace_access import (
|
||||||
@@ -187,95 +187,29 @@ class TestBuildDreamPrompt:
|
|||||||
|
|
||||||
|
|
||||||
class TestDreamRunCompletion:
|
class TestDreamRunCompletion:
|
||||||
"""DreamRunProgress + dream_run_completed gate cursor advancement."""
|
"""The runner's terminal state gates Dream cursor advancement."""
|
||||||
|
|
||||||
class _Resp:
|
class _Resp:
|
||||||
def __init__(self, stop_reason: str = "completed") -> None:
|
def __init__(self, stop_reason: str = "completed") -> None:
|
||||||
self.metadata = {"_stop_reason": stop_reason}
|
self.metadata = {"_stop_reason": stop_reason}
|
||||||
|
|
||||||
@staticmethod
|
def test_completed_stop_reason_completes(self):
|
||||||
async def _feed(progress: DreamRunProgress, *batches: list[dict]) -> None:
|
assert MemoryStore.dream_run_completed(self._Resp())
|
||||||
for batch in batches:
|
|
||||||
await progress("", tool_events=batch)
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.parametrize(
|
||||||
async def test_clean_run_completes(self):
|
"stop_reason",
|
||||||
progress = DreamRunProgress()
|
["error", "tool_error", "max_iterations", "cancelled"],
|
||||||
await self._feed(
|
)
|
||||||
progress,
|
def test_non_completed_stop_reason_blocks(self, stop_reason: str):
|
||||||
[{"phase": "start"}, {"phase": "end"}],
|
assert not MemoryStore.dream_run_completed(self._Resp(stop_reason))
|
||||||
)
|
|
||||||
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.asyncio
|
def test_missing_response_metadata_blocks(self):
|
||||||
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"))
|
|
||||||
assert not MemoryStore.dream_run_completed(None)
|
assert not MemoryStore.dream_run_completed(None)
|
||||||
|
|
||||||
def test_incompletion_reason_names_the_cause(self):
|
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(
|
assert MemoryStore.dream_incompletion_reason(
|
||||||
self._Resp(), had_tool_errors=True, recovered_tool_errors=True,
|
self._Resp("max_iterations")
|
||||||
) == "stop_reason: completed"
|
) == "stop_reason: max_iterations"
|
||||||
assert MemoryStore.dream_incompletion_reason(None) == (
|
assert MemoryStore.dream_incompletion_reason(None) == (
|
||||||
"stop_reason: missing response metadata"
|
"stop_reason: missing response metadata"
|
||||||
)
|
)
|
||||||
@@ -602,6 +536,45 @@ class TestEphemeralDirect:
|
|||||||
assert resp.metadata["_stop_reason"] == "error"
|
assert resp.metadata["_stop_reason"] == "error"
|
||||||
assert MemoryStore.dream_run_completed(resp) is False
|
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):
|
async def test_dream_turn_can_skip_unbatched_recent_history(self, tmp_path):
|
||||||
"""Dream must only see the batch selected by build_dream_prompt."""
|
"""Dream must only see the batch selected by build_dream_prompt."""
|
||||||
from unittest.mock import MagicMock
|
from unittest.mock import MagicMock
|
||||||
|
|||||||
@@ -274,8 +274,8 @@ async def test_dream_keeps_cursor_when_incomplete_with_diff(tmp_path) -> None:
|
|||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_dream_keeps_cursor_when_completed_after_tool_error(tmp_path) -> None:
|
async def test_dream_advances_cursor_when_completed_after_tool_error(tmp_path) -> None:
|
||||||
"""A soft tool failure must not masquerade as a verified no-op."""
|
"""A handled tool failure does not invalidate a normally completed run."""
|
||||||
ctx, store = _build_runnable_dream(
|
ctx, store = _build_runnable_dream(
|
||||||
tmp_path,
|
tmp_path,
|
||||||
initialized=True,
|
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 cmd_dream(ctx)
|
||||||
await asyncio.sleep(0)
|
await asyncio.sleep(0)
|
||||||
assert store._last_dream_cursor == 5
|
assert store._last_dream_cursor == 42
|
||||||
assert "did not complete" in ctx.loop.bus.outbound[0].content
|
assert "no memory changes" in ctx.loop.bus.outbound[0].content
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
@@ -339,7 +339,7 @@ async def test_dream_noop_batch_unlocks_following_history(tmp_path) -> None:
|
|||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_dream_non_git_falls_back_to_completion_gate(tmp_path) -> None:
|
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(
|
ctx, store = _build_runnable_dream(
|
||||||
tmp_path, initialized=False, content_diff="", stop_reason="completed",
|
tmp_path, initialized=False, content_diff="", stop_reason="completed",
|
||||||
)
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user