-
Notifications
You must be signed in to change notification settings - Fork 0
feat(state): Hermes-style human rewind for shadow checkpoints #98
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
4a273b6
9448905
221f890
b83d3b3
586b335
878ef1b
7d6a950
9516606
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,6 +72,11 @@ | |
| if TYPE_CHECKING: | ||
| from dream.context import ContextBreakdown | ||
| from dream.engine._engine import QueryEngine | ||
| from dream.state.shadow import ( | ||
| CheckpointSnapshot, | ||
| CombinedRestoreResult, | ||
| ShadowCheckpointManager, | ||
| ) | ||
|
|
||
|
|
||
| def _zero_cost() -> SessionCostSnapshot: | ||
|
|
@@ -165,6 +170,7 @@ def __init__( | |
| # path is missing. Resumed sessions receive the exact loaded revision. | ||
| self._snapshot_revision = _snapshot_revision | ||
| self._transcript: list[ConversationMessage] = [] | ||
| self._prompt_indices: list[int] = [] | ||
| self._cancel_event: asyncio.Event | None = None | ||
| self._closed = False | ||
| # Single-flight guard: ``Session`` keeps per-call cancel state on the | ||
|
|
@@ -231,6 +237,80 @@ def transcript(self) -> list[ConversationMessage]: | |
| """ | ||
| return self._transcript | ||
|
|
||
| @property | ||
| def checkpoint_manager(self) -> ShadowCheckpointManager | None: | ||
| """Shadow FS checkpoint manager when the bound engine was built with one.""" | ||
| engine = self._engine | ||
| if engine is None: | ||
| return None | ||
| return engine.checkpoint_manager | ||
|
|
||
| def list_checkpoints(self) -> list[CheckpointSnapshot]: | ||
| """List shadow checkpoints for this session's working directory (newest first).""" | ||
| engine = self._engine | ||
| manager = self.checkpoint_manager | ||
| if engine is None or manager is None: | ||
| return [] | ||
| return manager.list_for(engine.working_dir) | ||
|
|
||
| def restore_checkpoint( | ||
| self, | ||
| commit_sha: str | None = None, | ||
| *, | ||
| rewind_turns: int = 1, | ||
| ) -> CombinedRestoreResult: | ||
| """Hermes-style human rewind: restore FS and truncate the transcript. | ||
|
Comment on lines
+261
to
+262
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If another session is executing a mutating tool in the same harness worktree, this per-session Prompt To Fix With AIThis is a comment left during a code review.
Path: src/dream/session.py
Line: 207-208
Comment:
**Restore lacks worktree synchronization**
If another session is executing a mutating tool in the same harness worktree, this per-session `_active` check still permits a restore. Because the shared manager has no worktree-level lock, the reset can overwrite or interleave with that session's writes, leaving its transcript inconsistent with the resulting filesystem.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly. |
||
|
|
||
| ``commit_sha=None`` selects the newest checkpoint. Refuses while a | ||
| ``send`` is in flight so restore cannot race the live turn loop. | ||
| Turn boundaries come from ``Session`` (prompt submit indices), not | ||
| from inspecting message roles — engine-generated user messages are | ||
| not human turns. | ||
| """ | ||
| from dream.state.shadow import CombinedRestoreResult, RestoreOutcome, RestoreResult | ||
|
|
||
| if self._active: | ||
| raise RuntimeError("cannot restore checkpoint while a send is in flight") | ||
| engine = self._engine | ||
| manager = self.checkpoint_manager | ||
| if engine is None or manager is None: | ||
| return CombinedRestoreResult( | ||
| fs=RestoreResult( | ||
| outcome=RestoreOutcome.DISABLED, | ||
| detail="no checkpoint manager bound on this session", | ||
| ), | ||
| messages=tuple(self._transcript), | ||
| transcript_removed=0, | ||
| ) | ||
|
|
||
| sha = commit_sha | ||
| if sha is None: | ||
| listed = manager.list_for(engine.working_dir) | ||
| if not listed: | ||
| return CombinedRestoreResult( | ||
| fs=RestoreResult( | ||
| outcome=RestoreOutcome.NOT_FOUND, | ||
| detail="no checkpoints for working directory", | ||
| ), | ||
|
Comment on lines
+288
to
+294
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When two sessions on the harness create interleaved checkpoints, Prompt To Fix With AIThis is a comment left during a code review.
Path: src/dream/session.py
Line: 231-237
Comment:
**Restore selects cross-session checkpoints**
When two sessions on the harness create interleaved checkpoints, `list_for` returns their shared working-directory history and this branch selects the globally newest snapshot. The filesystem can therefore be restored to the other session's state while only the calling session's transcript is rewound, leaving chat and disk unrelated.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly. |
||
| messages=tuple(self._transcript), | ||
| transcript_removed=0, | ||
| ) | ||
| sha = listed[0].commit_sha | ||
|
|
||
| result = manager.restore_and_rewind( | ||
| engine.working_dir, | ||
| commit_sha=sha, | ||
| messages=self._transcript, | ||
| prompt_indices=self._prompt_indices, | ||
| rewind_turns=rewind_turns, | ||
| ) | ||
| if result.fs.outcome is RestoreOutcome.RESTORED: | ||
| self._transcript[:] = list(result.messages) | ||
| self._prompt_indices = [ | ||
| index for index in self._prompt_indices if index < len(self._transcript) | ||
| ] | ||
| return result | ||
|
|
||
| def _current_cost(self) -> SessionCostSnapshot: | ||
| return cost_snapshot_from_fields( | ||
| SessionCostFields( | ||
|
|
@@ -324,6 +404,7 @@ def restore_from_snapshot(self, snapshot: SessionSnapshot) -> None: | |
| """Replace transcript and cost counters from a saved snapshot.""" | ||
| restored = messages_from_records(snapshot.messages) | ||
| self._transcript[:] = sanitize_conversation_messages(restored) | ||
| self._prompt_indices.clear() | ||
| self.cost.input_tokens = snapshot.cost.input_tokens | ||
| self.cost.output_tokens = snapshot.cost.output_tokens | ||
| self.cost.cache_read_tokens = snapshot.cost.cache_read_tokens | ||
|
|
@@ -359,6 +440,7 @@ async def send(self, prompt: str) -> AsyncIterator[Event]: | |
| resume = list(self._transcript) if self._transcript else None | ||
| user_msg = ConversationMessage(role="user", content=[TextBlock(text=prompt)]) | ||
| self._transcript.append(user_msg) | ||
| self._prompt_indices.append(len(self._transcript) - 1) | ||
|
|
||
| config = self._engine.make_session_config() | ||
| if self._has_sent: | ||
|
|
@@ -537,6 +619,7 @@ def _apply_compaction(self) -> None: | |
| carryover = engine.carryover_metadata | ||
| if carryover is not None and carryover.last_compacted_transcript is not None: | ||
| self._transcript[:] = list(carryover.last_compacted_transcript) | ||
| self._prompt_indices.clear() | ||
| carryover.last_compacted_transcript = None | ||
| return | ||
| compactor = engine.compactor | ||
|
|
@@ -558,6 +641,7 @@ def _apply_compaction(self) -> None: | |
| ) | ||
| if result is not None: | ||
| self._transcript[:] = new_transcript | ||
| self._prompt_indices.clear() | ||
|
|
||
| async def cancel(self) -> None: | ||
| """Cancel the in-flight ``send``, if any. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,9 +32,14 @@ def __init__( | |
| self._manager = manager | ||
| self._working_dir = working_dir | ||
|
|
||
| @staticmethod | ||
| def _session_id(payload: Mapping[str, Any]) -> str | None: | ||
| raw_session_id = payload.get("session_id") | ||
| return str(raw_session_id) if raw_session_id is not None else None | ||
|
|
||
| async def __call__(self, event: HookEvent, payload: Mapping[str, Any]) -> HookResult: | ||
| if event is HookEvent.USER_PROMPT_SUBMIT: | ||
| self._manager.begin_turn() | ||
| self._manager.begin_turn(self._session_id(payload)) | ||
| return HookResult() | ||
|
|
||
| if event is not HookEvent.PRE_TOOL_USE: | ||
|
|
@@ -45,7 +50,11 @@ async def __call__(self, event: HookEvent, payload: Mapping[str, Any]) -> HookRe | |
| if reason is None: | ||
| return HookResult() | ||
|
|
||
| self._manager.ensure(self._working_dir, reason=reason) | ||
| self._manager.ensure( | ||
| self._working_dir, | ||
| reason=reason, | ||
| session_id=self._session_id(payload), | ||
| ) | ||
|
Comment on lines
+53
to
+57
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/dream/state/shadow/_hook.py
Line: 53-57
Comment:
**Session dedup key is lost**
`PRE_TOOL_USE` payloads do not contain `session_id`, so this call passes `None` and all sessions share a deduplication set that real-session `begin_turn` calls never clear. After the first conclusive checkpoint, subsequent mutations return `ALREADY_THIS_TURN` and no longer receive recoverable pre-mutation checkpoints.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly. |
||
| return HookResult() | ||
|
|
||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.