Skip to content

Commit e8af8c2

Browse files
yogthosYogthos
andauthored
feat(phase 2): sibling-branch pruning on compress + rewind with notification (#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>
1 parent d209366 commit e8af8c2

3 files changed

Lines changed: 243 additions & 3 deletions

File tree

src/session/mod.rs

Lines changed: 178 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -550,7 +550,30 @@ impl Session {
550550
}
551551
}
552552

553+
/// Legacy wrapper retained for tests that don't care about the
554+
/// pruned-siblings count. New callers should use
555+
/// `compress_reporting` to surface a "discarded N branches"
556+
/// notification to the user when sibling subtrees are dropped.
557+
#[cfg(test)]
553558
pub fn compress(&mut self, summary: String, first_kept_index: usize, token_savings: u64) {
559+
let _ = self.compress_reporting(summary, first_kept_index, token_savings);
560+
}
561+
562+
/// Same as `compress` but returns the number of NON-active-path
563+
/// nodes that were pruned from the tree because their ancestor
564+
/// chain rooted at a dropped message. Active-path message drops
565+
/// aren't counted — they're expected and already visible to the
566+
/// user as the conversation history shrinking. The host uses this
567+
/// count to surface a "discarded N forked branches" notification
568+
/// (matches opencode's drop-with-truncation pattern from
569+
/// `session/compaction.ts:386-396` — sibling branches outside the
570+
/// preserved tail are gone, full stop).
571+
pub fn compress_reporting(
572+
&mut self,
573+
summary: String,
574+
first_kept_index: usize,
575+
token_savings: u64,
576+
) -> usize {
554577
// Bounds check — callers compute `first_kept_index` from a
555578
// reverse-scan of `messages` so it should always be in range,
556579
// but a buggy/racy caller could pass out-of-bounds. Clamp
@@ -593,7 +616,63 @@ impl Session {
593616
// the new summary node as the new root, and re-parent the
594617
// first kept node to point at the summary.
595618
self.ensure_back_compat_initialized();
596-
for id in &dropped_ids {
619+
620+
// Sibling-branch prune (Phase 2). When a dropped message has
621+
// children that live in a forked sibling branch (not on the
622+
// active path), those children — and their descendants —
623+
// would be left with a `parent` pointing at a removed id.
624+
// Walk the tree and add all such descendants to the prune
625+
// set, then drop the union from tree + store. Tracks the
626+
// count of NON-active-path nodes removed so the host can
627+
// notify the user.
628+
//
629+
// Active-path messages (the ones still in `self.messages`
630+
// after drain + summary insert) are EXCLUDED from the
631+
// prune set — they'd otherwise get caught because the new
632+
// first-kept message's parent is still pointing at a dropped
633+
// id at this point in the function. The re-parent to summary
634+
// happens further below.
635+
let active_ids: std::collections::HashSet<CompactString> =
636+
self.messages.iter().map(|m| m.id.clone()).collect();
637+
638+
// Algorithm: fixed-point iteration. Start with the directly-
639+
// dropped ids. For each pass, find any tree entry whose
640+
// parent is in the prune set AND which is not on the active
641+
// path, then add it. Repeat until a pass adds nothing.
642+
// O(N * K) worst case where K = avg branch depth; typically
643+
// tiny in practice.
644+
let mut to_prune: std::collections::HashSet<CompactString> = dropped_set.clone();
645+
loop {
646+
let new_ids: Vec<CompactString> = self
647+
.tree
648+
.entries
649+
.iter()
650+
.filter(|(id, node)| {
651+
!to_prune.contains(id.as_str())
652+
&& !active_ids.contains(id.as_str())
653+
&& node
654+
.parent
655+
.as_ref()
656+
.map(|p| to_prune.contains(p))
657+
.unwrap_or(false)
658+
})
659+
.map(|(id, _)| id.clone())
660+
.collect();
661+
if new_ids.is_empty() {
662+
break;
663+
}
664+
for id in new_ids {
665+
to_prune.insert(id);
666+
}
667+
}
668+
669+
// Count of pruned nodes that were NOT on the active-path
670+
// (the dropped messages). Active drops are visible to the
671+
// user as the chat shrinking; sibling drops are silent
672+
// without a notification, hence the count.
673+
let sibling_pruned_count = to_prune.len().saturating_sub(dropped_set.len());
674+
675+
for id in &to_prune {
597676
self.tree.entries.remove(id);
598677
self.message_store.remove(id);
599678
}
@@ -653,6 +732,7 @@ impl Session {
653732
});
654733

655734
self.updated_at = CompactString::new(chrono::Utc::now().to_rfc3339());
735+
sibling_pruned_count
656736
}
657737
}
658738

@@ -1163,6 +1243,103 @@ mod tests {
11631243
assert_eq!(s.messages.len(), 1);
11641244
}
11651245

1246+
/// Phase 2 — compress drops a parent that has a sibling branch
1247+
/// underneath. The sibling subtree must also be pruned;
1248+
/// otherwise its nodes have `parent` pointing at a removed id
1249+
/// and the tree is dangling. Returns the count of pruned
1250+
/// non-active nodes so the host can surface a "discarded N
1251+
/// branches" notification.
1252+
#[test]
1253+
fn compress_prunes_sibling_branches_rooted_at_dropped_messages() {
1254+
let mut s = Session::new("p", "m", 0);
1255+
// Linear active branch: u1 → a1 → u2 → a2.
1256+
s.add_message(MessageRole::User, "u1");
1257+
let u1_id = s.messages.last().unwrap().id.clone();
1258+
s.add_message(MessageRole::Assistant, "a1");
1259+
s.add_message(MessageRole::User, "u2");
1260+
s.add_message(MessageRole::Assistant, "a2");
1261+
// Manually graft a sibling branch under u1: sib1 (child of
1262+
// u1) → sib2 (child of sib1). These live in tree.entries +
1263+
// message_store but NOT in `messages` (different branch).
1264+
let sib1_id = CompactString::new("sib1");
1265+
let sib2_id = CompactString::new("sib2");
1266+
s.tree.entries.insert(
1267+
sib1_id.clone(),
1268+
TreeNode {
1269+
id: sib1_id.clone(),
1270+
parent: Some(u1_id.clone()),
1271+
timestamp: 0,
1272+
label: None,
1273+
},
1274+
);
1275+
s.tree.entries.insert(
1276+
sib2_id.clone(),
1277+
TreeNode {
1278+
id: sib2_id.clone(),
1279+
parent: Some(sib1_id.clone()),
1280+
timestamp: 0,
1281+
label: None,
1282+
},
1283+
);
1284+
s.message_store.insert(
1285+
sib1_id.clone(),
1286+
SessionMessage {
1287+
role: MessageRole::Assistant,
1288+
content: CompactString::from("sib1"),
1289+
estimated_tokens: 1,
1290+
id: sib1_id.clone(),
1291+
timestamp: 0,
1292+
},
1293+
);
1294+
s.message_store.insert(
1295+
sib2_id.clone(),
1296+
SessionMessage {
1297+
role: MessageRole::Assistant,
1298+
content: CompactString::from("sib2"),
1299+
estimated_tokens: 1,
1300+
id: sib2_id.clone(),
1301+
timestamp: 0,
1302+
},
1303+
);
1304+
1305+
// Compress drops u1+a1 (first 2 messages). Sibling branch
1306+
// is rooted at u1 (a dropped id), so sib1 + sib2 must be
1307+
// pruned from tree + store along with the dropped messages.
1308+
let pruned = s.compress_reporting("summary".to_string(), 2, 10);
1309+
1310+
// u1, a1 (linear), sib1, sib2 (sibling) all gone from tree.
1311+
assert!(!s.tree.entries.contains_key(&u1_id), "u1 still in tree");
1312+
assert!(!s.tree.entries.contains_key(&sib1_id), "sib1 still in tree");
1313+
assert!(!s.tree.entries.contains_key(&sib2_id), "sib2 still in tree");
1314+
assert!(
1315+
!s.message_store.contains_key(&sib1_id),
1316+
"sib1 still in store",
1317+
);
1318+
assert!(
1319+
!s.message_store.contains_key(&sib2_id),
1320+
"sib2 still in store",
1321+
);
1322+
1323+
// Report: 2 sibling branch nodes were pruned (sib1, sib2).
1324+
// The linear u1+a1 drops aren't counted as "branches" since
1325+
// they were on the active path.
1326+
assert_eq!(pruned, 2, "expected 2 sibling nodes pruned, got {pruned}",);
1327+
}
1328+
1329+
/// When compress drops messages but there are NO sibling branches,
1330+
/// the report says 0 sibling nodes pruned. Confirms the counter
1331+
/// isn't accidentally counting active-path nodes.
1332+
#[test]
1333+
fn compress_reports_zero_pruned_when_no_siblings() {
1334+
let mut s = Session::new("p", "m", 0);
1335+
s.add_message(MessageRole::User, "u1");
1336+
s.add_message(MessageRole::Assistant, "a1");
1337+
s.add_message(MessageRole::User, "u2");
1338+
s.add_message(MessageRole::Assistant, "a2");
1339+
let pruned = s.compress_reporting("summary".to_string(), 2, 10);
1340+
assert_eq!(pruned, 0);
1341+
}
1342+
11661343
/// When given a parent session id, `reset_to_new` records it
11671344
/// in `name` so the prior session is reachable via session search.
11681345
#[test]

src/ui/mod.rs

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3230,16 +3230,60 @@ fn rewind_session(
32303230
.map(|m| m.id.clone())
32313231
.collect();
32323232
session.messages.truncate(msg_idx);
3233-
for id in &dropped_ids {
3233+
3234+
// Sibling-branch prune (Phase 2). Same logic as compress —
3235+
// walk descendants of dropped ids and remove any forked
3236+
// subtrees rooted on them. Active-path messages (still in
3237+
// `session.messages` after truncate) are excluded.
3238+
let dropped_set: std::collections::HashSet<_> = dropped_ids.iter().cloned().collect();
3239+
let active_ids: std::collections::HashSet<_> =
3240+
session.messages.iter().map(|m| m.id.clone()).collect();
3241+
let mut to_prune = dropped_set.clone();
3242+
loop {
3243+
let new_ids: Vec<_> = session
3244+
.tree
3245+
.entries
3246+
.iter()
3247+
.filter(|(id, node)| {
3248+
!to_prune.contains(*id)
3249+
&& !active_ids.contains(*id)
3250+
&& node
3251+
.parent
3252+
.as_ref()
3253+
.map(|p| to_prune.contains(p))
3254+
.unwrap_or(false)
3255+
})
3256+
.map(|(id, _)| id.clone())
3257+
.collect();
3258+
if new_ids.is_empty() {
3259+
break;
3260+
}
3261+
for id in new_ids {
3262+
to_prune.insert(id);
3263+
}
3264+
}
3265+
let pruned_siblings = to_prune.len().saturating_sub(dropped_set.len());
3266+
for id in &to_prune {
32343267
session.tree.entries.remove(id);
32353268
session.message_store.remove(id);
32363269
}
3270+
32373271
// Re-anchor `leaf_id` to the new tail (or None if everything
32383272
// was dropped). Previously the leaf was left pointing at a
32393273
// dropped id, which made `/tree` show a phantom branch.
32403274
session.tree.leaf_id = session.messages.last().map(|m| m.id.clone());
32413275
session.total_estimated_tokens = session.messages.iter().map(|m| m.estimated_tokens).sum();
32423276
renderer.write_line(&format!("rewound {} message(s)", removed), theme::accent())?;
3277+
if pruned_siblings > 0 {
3278+
renderer.write_line(
3279+
&format!(
3280+
"discarded {} forked branch node{} rooted in the rewound region",
3281+
pruned_siblings,
3282+
if pruned_siblings == 1 { "" } else { "s" },
3283+
),
3284+
c_error(),
3285+
)?;
3286+
}
32433287
}
32443288
Ok(())
32453289
}

src/ui/slash.rs

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,13 @@ pub async fn handle_compress(
122122
.map(|m| m.estimated_tokens)
123123
.sum();
124124

125-
session.compress(summary, cut_idx, tokens_before);
125+
// `compress_reporting` returns the count of non-active-path
126+
// tree nodes (sibling branches) pruned. We notify the user
127+
// about that loss explicitly — without the notification a
128+
// branched session could silently lose forks during auto-
129+
// compaction. opencode (`session/compaction.ts:386-396`) drops
130+
// siblings silently; dirge prefers the explicit notification.
131+
let pruned_branches = session.compress_reporting(summary, cut_idx, tokens_before);
126132

127133
let model = client.completion_model(session.model.to_string());
128134
*agent = crate::provider::build_agent(
@@ -147,6 +153,19 @@ pub async fn handle_compress(
147153
renderer.write_line("prompt cleared (back to default behavior)", c_agent())?;
148154

149155
render_session(renderer, session, cli, cfg, context)?;
156+
if pruned_branches > 0 {
157+
// Tell the user the branched topology shrunk. Without this,
158+
// they'd notice missing forks in `/tree` without any
159+
// explanation.
160+
renderer.write_line(
161+
&format!(
162+
"discarded {} forked branch node{} that were rooted in the compressed region",
163+
pruned_branches,
164+
if pruned_branches == 1 { "" } else { "s" },
165+
),
166+
c_error(),
167+
)?;
168+
}
150169
renderer.write_line(
151170
&format!(
152171
"compressed {} messages (saved ~{} tokens)",

0 commit comments

Comments
 (0)