feat(phase 2): sibling-branch pruning on compress + rewind with notification - #70
Merged
Conversation
…ication Phase 2 of the 6-phase plan. Reference pattern: opencode's `packages/opencode/src/session/compaction.ts:386-396` — drop branches outside the preserved tail with no special preservation. dirge adds an explicit "discarded N forked branches" notification on top so the user sees the loss instead of finding it later in `/tree`. ## Problem When `Session::compress` (or `rewind_session`) drops messages from the active path, any sibling branches whose parent nodes were among the dropped ids end up with `tree.entries[child].parent` pointing at a removed id. Subsequent `switch_to_leaf` / `/tree` walks either fail silently or render dangling phantom branches. Before Phase 2: dropped messages removed from tree + store, but their forked descendants were left orphaned. ## Fix New `Session::compress_reporting(summary, cut_idx, savings) -> usize`: - Builds the to-drop set from active-path ids (`messages[..cut_idx]`). - Builds an EXCLUSION set of currently-active ids (messages still in `self.messages` after drain + summary insert) — without this the new first-kept message gets caught because its parent is still pointing at a dropped id at this point in the function. - Fixed-point walk: each pass finds tree entries whose parent is in `to_prune` but which aren't excluded, adds them; repeat until no change. O(N * K) where K = avg branch depth. - Prunes the union from `tree.entries` + `message_store`. - Returns the count of non-active-path nodes pruned (subtracts the direct dropped count) so the caller can notify. `Session::compress` becomes a thin `#[cfg(test)]` wrapper that calls `compress_reporting` and discards the count, kept for the 4 existing compress tests that didn't care about the count. Mirror the same walk in `ui::mod.rs::rewind_session` — both paths that prune active-path messages now also prune dependent siblings. `handle_compress` (slash.rs) renders a red `discarded N forked branch node(s) that were rooted in the compressed region` line when the count is non-zero. Same for rewind. opencode handles this silently; dirge surfaces it because branched sessions are intentional and a silent loss would surprise users. ## Tests 3 new session tests: - `compress_prunes_sibling_branches_rooted_at_dropped_messages` — builds a branched fixture (u1 → a1 → u2 → a2 with a sibling subtree `sib1 → sib2` rooted at u1), compresses past u1, asserts both sibs are gone from tree + store and the count is 2. - `compress_reports_zero_pruned_when_no_siblings` — linear session, count should be 0. Plus the existing rewind tests still pass (the new prune logic doesn't fire when there are no siblings, so linear rewind is unaffected). 639 pass total (was 637), 0 warnings across all build profiles. ## Test plan - [x] `cargo test --features plugin` -> 639 pass. - [x] `cargo build --all-features` -> 0 warnings. ## Up next: Phase 3 (tool-call structured persistence) When a session is resumed, prior tool calls + results are currently lost (only assistant text is in `SessionMessage`). The LLM sees text-only traces and may re-attempt the same tools. Phase 3 will add structured `tool_calls: Vec<ToolCallEntry>` to `SessionMessage` (opencode's `ToolPart` pattern with interrupted- state pairing for Anthropic compatibility).
yogthos
added a commit
that referenced
this pull request
May 21, 2026
) Phase 5 of the plan. No clean equivalent in opencode or pi — both manage allowlist entries only via the interactive permission prompt's "allow always" answer. dirge adds explicit CRUD so users can inspect, manually add, drop one, or clear all entries without editing the session JSON. ## Commands - `/allow` or `/allow list` — show numbered entries - `/allow add <tool> <pattern>` — add a (tool, pattern) entry, e.g. `/allow add bash 'cargo *'` - `/allow remove <idx>` — drop one entry by 0-based index from the `list` output - `/allow clear` — drop all entries The `add` form preserves pattern strings containing spaces by re-deriving the args from the raw `text` slice (rather than relying on the SmallVec `parts[2]` which would get truncated at the first whitespace). Same approach `/cd` uses for paths. ## State sync Both surfaces stay aligned: - `PermissionChecker::session_allowlist` (in-memory, used by `check` / `check_path`) — updated via the existing `add_session_allowlist` and the two new `remove_session_allowlist_at(idx)` + `clear_session_allowlist`. - `Session::permission_allowlist` (persisted to JSON, restored on resume via `load_session_allowlist`) — mirrored manually in the slash handler so save/load round-trips show the user-edited list. Without the session-side mirror, a `/allow add foo bar` would stay in memory for this run but vanish on `-c` resume. Dedup at the session level too so `save()` writes a clean list. ## Tests 3 new permission tests, written failing first: - `remove_session_allowlist_at_returns_removed_entry`: remove(1) on [bash:cargo*, bash:git*, read:/tmp/*] yields Some(("bash","git *")) and the surviving entries shift down. - `remove_session_allowlist_at_out_of_range_returns_none`: remove(99) returns None without panicking; existing entries intact. - `clear_session_allowlist_empties_the_list`: clear() drops all entries. The slash handler itself is not unit-tested (requires the full UI loop fixtures); manual smoke test: ``` $ dirge --restrictive > bash 'echo hi' > (a) allow always > /allow list → [0] bash echo * > /allow add read /tmp/* > /allow list → [0] bash echo * [1] read /tmp/* > /allow remove 0 > /allow list → [0] read /tmp/* > /allow clear > /allow list → empty ``` ## Docs `/help` text gets four new lines covering each subcommand. ## Test plan - [x] `cargo test --features plugin` -> 649 pass (was 646). - [x] `cargo build --all-features` -> compiles. - [x] `cargo build --no-default-features` -> compiles. ## Plan status | Phase | PR | Done | |---|---|---| | 1 | #69 | first-wins block + docs | | 2 | #70 | sibling-branch pruning + notification | | 3 | #71 | structured tool-call persistence | | 4 | #72 | branch summary metadata in /tree | | 5 | this | /allow CRUD | | 6 | — | skipped per user request (cost tracking) | 5 of 6 phases complete; Phase 6 (cost tracking) skipped. Co-authored-by: Yogthos <yogthos@gmail.com>
allen-munsch
pushed a commit
to allen-munsch/dirge
that referenced
this pull request
Jun 3, 2026
…ication (dirge-code#70) Phase 2 of the 6-phase plan. Reference pattern: opencode's `packages/opencode/src/session/compaction.ts:386-396` — drop branches outside the preserved tail with no special preservation. dirge adds an explicit "discarded N forked branches" notification on top so the user sees the loss instead of finding it later in `/tree`. ## Problem When `Session::compress` (or `rewind_session`) drops messages from the active path, any sibling branches whose parent nodes were among the dropped ids end up with `tree.entries[child].parent` pointing at a removed id. Subsequent `switch_to_leaf` / `/tree` walks either fail silently or render dangling phantom branches. Before Phase 2: dropped messages removed from tree + store, but their forked descendants were left orphaned. ## Fix New `Session::compress_reporting(summary, cut_idx, savings) -> usize`: - Builds the to-drop set from active-path ids (`messages[..cut_idx]`). - Builds an EXCLUSION set of currently-active ids (messages still in `self.messages` after drain + summary insert) — without this the new first-kept message gets caught because its parent is still pointing at a dropped id at this point in the function. - Fixed-point walk: each pass finds tree entries whose parent is in `to_prune` but which aren't excluded, adds them; repeat until no change. O(N * K) where K = avg branch depth. - Prunes the union from `tree.entries` + `message_store`. - Returns the count of non-active-path nodes pruned (subtracts the direct dropped count) so the caller can notify. `Session::compress` becomes a thin `#[cfg(test)]` wrapper that calls `compress_reporting` and discards the count, kept for the 4 existing compress tests that didn't care about the count. Mirror the same walk in `ui::mod.rs::rewind_session` — both paths that prune active-path messages now also prune dependent siblings. `handle_compress` (slash.rs) renders a red `discarded N forked branch node(s) that were rooted in the compressed region` line when the count is non-zero. Same for rewind. opencode handles this silently; dirge surfaces it because branched sessions are intentional and a silent loss would surprise users. ## Tests 3 new session tests: - `compress_prunes_sibling_branches_rooted_at_dropped_messages` — builds a branched fixture (u1 → a1 → u2 → a2 with a sibling subtree `sib1 → sib2` rooted at u1), compresses past u1, asserts both sibs are gone from tree + store and the count is 2. - `compress_reports_zero_pruned_when_no_siblings` — linear session, count should be 0. Plus the existing rewind tests still pass (the new prune logic doesn't fire when there are no siblings, so linear rewind is unaffected). 639 pass total (was 637), 0 warnings across all build profiles. ## Test plan - [x] `cargo test --features plugin` -> 639 pass. - [x] `cargo build --all-features` -> 0 warnings. ## Up next: Phase 3 (tool-call structured persistence) When a session is resumed, prior tool calls + results are currently lost (only assistant text is in `SessionMessage`). The LLM sees text-only traces and may re-attempt the same tools. Phase 3 will add structured `tool_calls: Vec<ToolCallEntry>` to `SessionMessage` (opencode's `ToolPart` pattern with interrupted- state pairing for Anthropic compatibility). Co-authored-by: Yogthos <yogthos@gmail.com>
allen-munsch
pushed a commit
to allen-munsch/dirge
that referenced
this pull request
Jun 3, 2026
…irge-code#73) Phase 5 of the plan. No clean equivalent in opencode or pi — both manage allowlist entries only via the interactive permission prompt's "allow always" answer. dirge adds explicit CRUD so users can inspect, manually add, drop one, or clear all entries without editing the session JSON. ## Commands - `/allow` or `/allow list` — show numbered entries - `/allow add <tool> <pattern>` — add a (tool, pattern) entry, e.g. `/allow add bash 'cargo *'` - `/allow remove <idx>` — drop one entry by 0-based index from the `list` output - `/allow clear` — drop all entries The `add` form preserves pattern strings containing spaces by re-deriving the args from the raw `text` slice (rather than relying on the SmallVec `parts[2]` which would get truncated at the first whitespace). Same approach `/cd` uses for paths. ## State sync Both surfaces stay aligned: - `PermissionChecker::session_allowlist` (in-memory, used by `check` / `check_path`) — updated via the existing `add_session_allowlist` and the two new `remove_session_allowlist_at(idx)` + `clear_session_allowlist`. - `Session::permission_allowlist` (persisted to JSON, restored on resume via `load_session_allowlist`) — mirrored manually in the slash handler so save/load round-trips show the user-edited list. Without the session-side mirror, a `/allow add foo bar` would stay in memory for this run but vanish on `-c` resume. Dedup at the session level too so `save()` writes a clean list. ## Tests 3 new permission tests, written failing first: - `remove_session_allowlist_at_returns_removed_entry`: remove(1) on [bash:cargo*, bash:git*, read:/tmp/*] yields Some(("bash","git *")) and the surviving entries shift down. - `remove_session_allowlist_at_out_of_range_returns_none`: remove(99) returns None without panicking; existing entries intact. - `clear_session_allowlist_empties_the_list`: clear() drops all entries. The slash handler itself is not unit-tested (requires the full UI loop fixtures); manual smoke test: ``` $ dirge --restrictive > bash 'echo hi' > (a) allow always > /allow list → [0] bash echo * > /allow add read /tmp/* > /allow list → [0] bash echo * [1] read /tmp/* > /allow remove 0 > /allow list → [0] read /tmp/* > /allow clear > /allow list → empty ``` ## Docs `/help` text gets four new lines covering each subcommand. ## Test plan - [x] `cargo test --features plugin` -> 649 pass (was 646). - [x] `cargo build --all-features` -> compiles. - [x] `cargo build --no-default-features` -> compiles. ## Plan status | Phase | PR | Done | |---|---|---| | 1 | dirge-code#69 | first-wins block + docs | | 2 | dirge-code#70 | sibling-branch pruning + notification | | 3 | dirge-code#71 | structured tool-call persistence | | 4 | dirge-code#72 | branch summary metadata in /tree | | 5 | this | /allow CRUD | | 6 | — | skipped per user request (cost tracking) | 5 of 6 phases complete; Phase 6 (cost tracking) skipped. Co-authored-by: Yogthos <yogthos@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 2 of the 6-phase plan. Pruning algorithm mirrors opencode's drop-with-truncation (no fancy preservation), but adds an explicit chat notification when forked branches are discarded so the loss isn't silent. New
Session::compress_reportingreturns count of sibling nodes pruned. Same algorithm reused inrewind_session. 3 new tests, 639 pass.