Merge pull request #597 from sanyamsingh001/fix/history-trim

crates/engine/src/history.rs

@@ -206,19 +206,26 @@ impl History {
         self.branches.push(Branch { parent: from.clone(), steps });
     }
 
-    /// Apply the undo-levels limit: drop the oldest working-line states, the branches that grew
-    /// from them, and the oldest branches when branches hold more states than `levels`.
+    /// Apply the undo-levels limit: keep the nearest past and future states on the working
+    /// line, drop branches whose parents were removed, and bound branch states separately.
     pub fn trim(&mut self, levels: usize, current: &Arc<Project>) {
         let levels = levels.max(1);
         if self.undo.len() > levels {
             let extra = self.undo.len() - levels;
             self.undo.drain(..extra);
         }
-        let mut total: usize = self.branches.iter().map(|b| b.steps.len()).sum();
-        while total > levels && !self.branches.is_empty() {
-            total -= self.branches.remove(0).steps.len();
+        if self.redo.len() > levels {
+            // The next redo is last: discard the farthest future states first.
+            let extra = self.redo.len() - levels;
+            self.redo.drain(..extra);
         }
+        // Orphans from truncated line states must not consume the reachable branch budget.
         self.prune(current);
+        while self.branch_states() > levels && !self.branches.is_empty() {
+            self.branches.remove(0);
+            // Removing a parent branch also removes its descendants before counting again.
+            self.prune(current);
+        }
     }
 
     /// Drop branches whose parent state is no longer in the history (`current` = the current

crates/engine/src/tests_history.rs

@@ -1,9 +1,13 @@
 //! Branching history: undo then a new step keeps the undone line as a branch; the tree lists
 //! every state; jumping to any state (on any branch) restores it and keeps everything else.
 
+use std::sync::Arc;
+
+use effectcraft_project::Project;
 use serde_json::{Value, json};
 
 use crate::Session;
+use crate::history::{Branch, History};
 
 fn names(s: &Session) -> Vec<String> {
     s.project.comps().next().map(|(_, c)| c.layers.iter().map(|l| l.name.clone()).collect()).unwrap_or_default()
@@ -190,3 +194,46 @@ fn a_session_restored_with_unsaved_changes_stays_modified() {
     s.execute("edit.undo", json!({})).unwrap();
     assert!(s.is_dirty(), "undo doesn't reach a saved state");
 }
+
+#[test]
+fn dropped_redo_parent_does_not_discard_reachable_branches() {
+    let state = || Arc::new(Project::default());
+    let current = state();
+    let futures: Vec<_> = (0..5).map(|_| state()).collect();
+    let reachable = state();
+    let mut history = History {
+        redo: futures.iter().rev().map(|p| ("future".into(), p.clone())).collect(),
+        branches: vec![
+            Branch { parent: current.clone(), steps: vec![("reachable".into(), reachable.clone())] },
+            Branch { parent: futures[4].clone(), steps: vec![("orphan1".into(), state()), ("orphan2".into(), state())] },
+        ],
+        ..Default::default()
+    };
+    history.trim(2, &current);
+    assert_eq!(history.redo.len(), 2);
+    assert_eq!(history.branches.len(), 1);
+    assert!(Arc::ptr_eq(&history.branches[0].steps[0].1, &reachable));
+    assert_eq!(history.branch_states(), 1);
+    assert!(history.tree(&current).iter().any(|node| node.label == "reachable"));
+}
+
+#[test]
+fn evicted_branch_parent_does_not_discard_other_reachable_branches() {
+    let state = || Arc::new(Project::default());
+    let current = state();
+    let parent = state();
+    let reachable = state();
+    let mut history = History {
+        branches: vec![
+            Branch { parent: current.clone(), steps: vec![("parent".into(), parent.clone())] },
+            Branch { parent: current.clone(), steps: vec![("reachable".into(), reachable.clone())] },
+            Branch { parent, steps: vec![("orphan1".into(), state()), ("orphan2".into(), state())] },
+        ],
+        ..Default::default()
+    };
+    history.trim(2, &current);
+    assert_eq!(history.branches.len(), 1);
+    assert!(Arc::ptr_eq(&history.branches[0].steps[0].1, &reachable));
+    assert_eq!(history.branch_states(), 1);
+    assert_eq!(history.tree(&current).len(), 2);
+}