fix(dream): ground memory audit records in the real git diff (#4673)
* fix(dream): ground commit messages and cursor advance in the real git diff Dream consolidation could emit a /dream-log audit record that did not match the actual file changes: build_dream_commit_message appended the LLM's unverified resp.content, dream_run_completed only checked the stop reason, and file contents were deliberately omitted from the prompt. The combination let a single-turn self-report become the durable audit record. - gitstore: add summarize_working_tree() — a structured, machine-derived summary (per-file +N/-M, totals, capped unified diff) of working-tree changes vs HEAD. Pure filesystem/git ground truth, never LLM narrative. - memory: build_dream_commit_message now takes the diff body instead of resp; dream_content_diff() exposes the real delta over SOUL/USER/MEMORY.md only (excludes .dream_cursor so cursor writes aren't mistaken for edits); build_dream_prompt embeds current file contents so the model edits reality, not a stale mental model. - builtin/cli: both Dream paths now compute the diff, gate cursor advance on a non-empty delta (no-op runs no longer swallow history), and commit with the diff-grounded message. Non-git workspaces fall back to the completion check. - dream.md: document that contents are embedded, and add a chain-of- verification guardrail so the model's summary cannot claim unmade edits. A regression test proves a lying resp.content never reaches the audit log while the real diff does. * fix(dream): mark non-UTF-8 memory files as binary in diff summary Address review feedback (Q1 on PR #4673): summarize_working_tree read working-tree files with errors="replace", which would emit U+FFFD replacement chars into the audit record if a memory file ever held invalid UTF-8 — misrepresenting the diff it is meant to make truthful. Switch to errors="strict" and catch UnicodeDecodeError: a non-UTF-8 (or binary/corrupt) file is now recorded as "{path}: binary or non-UTF-8 file changed" and omitted from the unified diff, so the audit record stays honest. An empty diff block is also suppressed when all changes are binary. Adds a defensive regression test asserting no replacement char leaks.
This commit is contained in:
Kenneth Zhao
+97
-27
@@ -61,6 +61,29 @@ class TestBuildDreamPrompt:
|
||||
prompt, _ = result
|
||||
assert "skill-creator" in prompt
|
||||
|
||||
def test_prompt_embeds_current_memory_file_contents(self, store):
|
||||
"""Dream must see the real current file contents (Tier 4) so it edits the
|
||||
files, not a stale mental model."""
|
||||
store.append_history("hello")
|
||||
result = store.build_dream_prompt()
|
||||
assert result is not None
|
||||
prompt, _ = result
|
||||
assert "## Current Memory Files" in prompt
|
||||
assert "### SOUL.md" in prompt
|
||||
assert "### USER.md" in prompt
|
||||
assert "### memory/MEMORY.md" in prompt
|
||||
# Real current contents are embedded verbatim.
|
||||
assert "Project X active" in prompt
|
||||
assert "Helpful" in prompt
|
||||
|
||||
def test_prompt_renders_missing_files_as_empty(self, tmp_path):
|
||||
store = MemoryStore(tmp_path) # no durable files written
|
||||
store.append_history("hello")
|
||||
result = store.build_dream_prompt()
|
||||
assert result is not None
|
||||
prompt, _ = result
|
||||
assert "(empty)" in prompt
|
||||
|
||||
def test_workspace_dream_prompt_overrides_default(self, store):
|
||||
store.dream_prompt_file.parent.mkdir(parents=True)
|
||||
store.dream_prompt_file.write_text(
|
||||
@@ -566,48 +589,95 @@ class TestEphemeralHooks:
|
||||
spy.before_iteration.assert_called()
|
||||
|
||||
class TestDreamCommitMessage:
|
||||
async def test_commit_includes_response_summary(self, tmp_path):
|
||||
"""Git auto-commit after Dream should include the LLM response in the body."""
|
||||
import subprocess
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
def test_commit_message_reflects_real_diff_not_narrative(self, tmp_path):
|
||||
"""The Dream commit message must mirror the real git diff and ignore the
|
||||
LLM's narrative, so ``/dream-log`` can never lie.
|
||||
|
||||
from nanobot.agent.memory import MemoryStore
|
||||
Regression for the hallucinated-commit bug: commit ``a72ca2a`` claimed a
|
||||
"Medical Research" section that never reached the diff.
|
||||
"""
|
||||
import subprocess
|
||||
|
||||
store = MemoryStore(tmp_path)
|
||||
store.write_soul("# Soul")
|
||||
store.write_memory("# Memory")
|
||||
store.append_history("user discussed project goals")
|
||||
|
||||
provider = MagicMock()
|
||||
provider.get_default_model.return_value = "test-model"
|
||||
provider.supports_tools = True
|
||||
provider.generation = MagicMock(max_tokens=4096)
|
||||
provider.chat_with_retry = AsyncMock(return_value=MagicMock(
|
||||
content="Identified 2 new facts about project goals",
|
||||
finish_reason="stop",
|
||||
tool_calls=[],
|
||||
usage={},
|
||||
))
|
||||
|
||||
store.git.init()
|
||||
store.git.auto_commit("initial state")
|
||||
|
||||
# Simulate what the cron handler does: produce a resp with content,
|
||||
# build the commit message via the actual function, then commit.
|
||||
resp_content = "Identified 2 new facts about project goals"
|
||||
resp = MagicMock(content=resp_content)
|
||||
# A real edit to a tracked content file.
|
||||
store.write_memory("# Memory\n- DMSO research notes")
|
||||
|
||||
# A lying narrative the old code would have appended verbatim.
|
||||
lying = "Added a Medical Research section (Mastic Gum, DMSO) to MEMORY.md"
|
||||
diff_body = store.dream_content_diff()
|
||||
assert diff_body, "real edit must be detected"
|
||||
assert "DMSO research notes" in diff_body
|
||||
assert lying not in diff_body
|
||||
|
||||
msg = MemoryStore.build_dream_commit_message(
|
||||
"dream: periodic memory consolidation", resp,
|
||||
"dream: periodic memory consolidation", diff_body,
|
||||
)
|
||||
assert lying not in msg
|
||||
assert "DMSO research notes" in msg
|
||||
|
||||
# Write a change so auto_commit has something to commit
|
||||
store.write_memory("# Memory\n- Updated by Dream")
|
||||
sha = store.git.auto_commit(msg)
|
||||
assert sha is not None
|
||||
|
||||
log = subprocess.check_output(
|
||||
["git", "log", "-1", "--format=%B"],
|
||||
cwd=str(tmp_path), text=True,
|
||||
).strip()
|
||||
assert "dream: periodic memory consolidation" in log
|
||||
assert "Identified 2 new facts" in log
|
||||
assert "DMSO research notes" in log
|
||||
assert lying not in log
|
||||
|
||||
def test_commit_message_is_bare_prefix_when_no_changes(self, tmp_path):
|
||||
"""A no-op Dream run yields only the prefix — never a narrated summary."""
|
||||
store = MemoryStore(tmp_path)
|
||||
store.write_soul("# Soul")
|
||||
store.write_memory("# Memory")
|
||||
store.git.init()
|
||||
store.git.auto_commit("initial state")
|
||||
|
||||
# No edits at all.
|
||||
assert store.dream_content_diff() == ""
|
||||
msg = MemoryStore.build_dream_commit_message(
|
||||
"dream: manual run", store.dream_content_diff(),
|
||||
)
|
||||
assert msg == "dream: manual run"
|
||||
|
||||
def test_build_commit_message_ignores_none_and_empty_body(self):
|
||||
assert MemoryStore.build_dream_commit_message("dream: x", "") == "dream: x"
|
||||
assert MemoryStore.build_dream_commit_message("dream: x", None) == "dream: x"
|
||||
assert MemoryStore.build_dream_commit_message("dream: x", " ") == "dream: x"
|
||||
assert (
|
||||
MemoryStore.build_dream_commit_message("dream: x", "SOUL.md: +1 -0")
|
||||
== "dream: x\n\nSOUL.md: +1 -0"
|
||||
)
|
||||
|
||||
|
||||
class TestDreamContentDiff:
|
||||
"""The ground-truth signal that gates cursor advance and commit messages."""
|
||||
|
||||
def test_empty_when_git_not_initialized(self, store):
|
||||
assert store.dream_content_diff() == ""
|
||||
|
||||
def test_empty_when_no_tracked_changes(self, store):
|
||||
store.git.init()
|
||||
store.git.auto_commit("initial")
|
||||
assert store.dream_content_diff() == ""
|
||||
|
||||
def test_reflects_real_content_edits(self, store):
|
||||
store.git.init()
|
||||
store.git.auto_commit("initial")
|
||||
store.write_memory("# Memory\n- DMSO research notes")
|
||||
diff = store.dream_content_diff()
|
||||
assert diff
|
||||
assert "memory/MEMORY.md" in diff
|
||||
assert "DMSO research notes" in diff
|
||||
|
||||
def test_ignores_cursor_only_changes(self, store):
|
||||
"""Advancing the cursor must not count as a productive content edit."""
|
||||
store.git.init()
|
||||
store.git.auto_commit("initial")
|
||||
store.set_last_dream_cursor(99) # only memory/.dream_cursor changes
|
||||
assert store.dream_content_diff() == ""
|
||||
|
||||
@@ -20,10 +20,17 @@ from nanobot.utils.gitstore import CommitInfo
|
||||
|
||||
|
||||
class _FakeStore:
|
||||
def __init__(self, git, last_dream_cursor: int = 1, dream_prompt_result=None):
|
||||
def __init__(
|
||||
self,
|
||||
git,
|
||||
last_dream_cursor: int = 1,
|
||||
dream_prompt_result=None,
|
||||
content_diff: str = "",
|
||||
):
|
||||
self.git = git
|
||||
self._last_dream_cursor = last_dream_cursor
|
||||
self._dream_prompt_result = dream_prompt_result
|
||||
self._content_diff = content_diff
|
||||
self.compact_history_called = False
|
||||
|
||||
def get_last_dream_cursor(self) -> int:
|
||||
@@ -38,6 +45,9 @@ class _FakeStore:
|
||||
def set_last_dream_cursor(self, value: int) -> None:
|
||||
self._last_dream_cursor = value
|
||||
|
||||
def dream_content_diff(self) -> str:
|
||||
return self._content_diff
|
||||
|
||||
def compact_history(self) -> None:
|
||||
self.compact_history_called = True
|
||||
|
||||
@@ -159,6 +169,74 @@ async def test_dream_internal_run_silences_progress(tmp_path) -> None:
|
||||
assert callable(calls[0][1]["on_progress"])
|
||||
|
||||
|
||||
def _build_runnable_dream(
|
||||
tmp_path,
|
||||
*,
|
||||
initialized: bool,
|
||||
content_diff: str,
|
||||
stop_reason: str = "completed",
|
||||
) -> tuple[CommandContext, _FakeStore]:
|
||||
"""Build a /dream ctx whose run is driven by a canned stop reason + diff."""
|
||||
msg = InboundMessage(channel="cli", sender_id="u1", chat_id="direct", content="/dream")
|
||||
store = _FakeStore(
|
||||
_FakeGit(initialized=initialized),
|
||||
last_dream_cursor=5,
|
||||
dream_prompt_result=("dream prompt", 42),
|
||||
content_diff=content_diff,
|
||||
)
|
||||
|
||||
async def process_direct(*args, **kwargs):
|
||||
return OutboundMessage(
|
||||
channel="cli",
|
||||
chat_id="direct",
|
||||
content="done",
|
||||
metadata={"_stop_reason": stop_reason},
|
||||
)
|
||||
|
||||
bus = _FakeBus()
|
||||
sessions_dir = tmp_path / "sessions"
|
||||
sessions_dir.mkdir()
|
||||
loop = SimpleNamespace(
|
||||
bus=bus,
|
||||
context=SimpleNamespace(memory=store, timezone="UTC"),
|
||||
sessions=SimpleNamespace(sessions_dir=sessions_dir),
|
||||
process_direct=process_direct,
|
||||
)
|
||||
ctx = CommandContext(msg=msg, session=None, key=msg.session_key, raw="/dream", args="", loop=loop)
|
||||
return ctx, store
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_dream_advances_cursor_when_diff_nonempty(tmp_path) -> None:
|
||||
"""A real file delta => productive run => cursor advances (Tier 3)."""
|
||||
ctx, store = _build_runnable_dream(tmp_path, initialized=True, content_diff="SOUL.md: +1 -0")
|
||||
await cmd_dream(ctx)
|
||||
await asyncio.sleep(0)
|
||||
assert store._last_dream_cursor == 42
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_dream_keeps_cursor_on_completed_noop(tmp_path) -> None:
|
||||
"""Completed run with no file changes must NOT advance the cursor, so the
|
||||
history batch is reconsidered next run instead of silently swallowed."""
|
||||
ctx, store = _build_runnable_dream(tmp_path, initialized=True, content_diff="")
|
||||
await cmd_dream(ctx)
|
||||
await asyncio.sleep(0)
|
||||
assert store._last_dream_cursor == 5 # unchanged
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_dream_non_git_falls_back_to_completion_gate(tmp_path) -> None:
|
||||
"""Without git there is no diff signal; productivity falls back to the
|
||||
completion check so non-git workspaces keep working."""
|
||||
ctx, store = _build_runnable_dream(
|
||||
tmp_path, initialized=False, content_diff="", stop_reason="completed",
|
||||
)
|
||||
await cmd_dream(ctx)
|
||||
await asyncio.sleep(0)
|
||||
assert store._last_dream_cursor == 42 # advanced via completion fallback
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_dream_log_latest_is_more_user_friendly() -> None:
|
||||
commit = CommitInfo(sha="abcd1234", message="dream: 2026-04-04, 2 change(s)", timestamp="2026-04-04 12:00")
|
||||
|
||||
@@ -98,6 +98,51 @@ class TestLineAges:
|
||||
assert age_by_line["- keep"] == 30
|
||||
|
||||
|
||||
class TestSummarizeWorkingTree:
|
||||
"""Ground-truth diff summary used to keep Dream audit records honest."""
|
||||
|
||||
def test_empty_when_not_initialized(self, tmp_path):
|
||||
git = GitStore(tmp_path, tracked_files=["MEMORY.md"])
|
||||
assert git.summarize_working_tree(["MEMORY.md"]) == ""
|
||||
|
||||
def test_empty_when_no_changes(self, git):
|
||||
assert git.summarize_working_tree(["MEMORY.md", "SOUL.md"]) == ""
|
||||
|
||||
def test_summarizes_real_change(self, git, tmp_path):
|
||||
(tmp_path / "MEMORY.md").write_text("# Memory\n- new fact\n", encoding="utf-8")
|
||||
summary = git.summarize_working_tree(["MEMORY.md"])
|
||||
assert "MEMORY.md: +2 -0" in summary
|
||||
assert "new fact" in summary
|
||||
assert "1 file changed, 2 insertions(+), 0 deletions(-)" in summary
|
||||
|
||||
def test_only_reports_requested_paths(self, git, tmp_path):
|
||||
# MEMORY.md changes, but we only ask about the unchanged SOUL.md.
|
||||
(tmp_path / "MEMORY.md").write_text("changed\n", encoding="utf-8")
|
||||
assert git.summarize_working_tree(["SOUL.md"]) == ""
|
||||
|
||||
def test_counts_additions_and_removals(self, git, tmp_path):
|
||||
(tmp_path / "MEMORY.md").write_text("# M\n- keep\n- new\n", encoding="utf-8")
|
||||
summary = git.summarize_working_tree(["MEMORY.md"])
|
||||
assert "MEMORY.md: +3 -0" in summary
|
||||
|
||||
def test_detects_deletion(self, git, tmp_path):
|
||||
# File removed from the working tree (must have content first; the
|
||||
# fixture's tracked files start empty, so an empty-file delete is a no-op).
|
||||
(tmp_path / "MEMORY.md").write_text("has content\n", encoding="utf-8")
|
||||
git.auto_commit("add content")
|
||||
(tmp_path / "MEMORY.md").unlink()
|
||||
summary = git.summarize_working_tree(["MEMORY.md"])
|
||||
assert summary # a removal is still a change
|
||||
assert "deletion" in summary
|
||||
|
||||
def test_non_utf8_file_marked_binary_without_replacement_chars(self, git, tmp_path):
|
||||
# Invalid UTF-8 must not leak replacement chars into the audit record.
|
||||
(tmp_path / "MEMORY.md").write_bytes(b"\x89PNG\r\n\x1a\n\xff\xfe\x00\x01")
|
||||
summary = git.summarize_working_tree(["MEMORY.md"])
|
||||
assert "MEMORY.md: binary or non-UTF-8 file changed" in summary
|
||||
assert "\ufffd" not in summary # no U+FFFD replacement chars leaked
|
||||
|
||||
|
||||
class TestNestedRepoProtection:
|
||||
"""Regression tests for GitHub issue #2980: nested repo protection."""
|
||||
|
||||
|
||||
Reference in New Issue
Block a user