fix diff generation and add test coverage

base/src/test/user_model/mod.rs

@@ -1,6 +1,7 @@
 mod test_add_delete_sheets;
 mod test_autofill_columns;
 mod test_autofill_rows;
+mod test_batch_row_column_diff;
 mod test_border;
 mod test_clear_cells;
 mod test_column_style;

base/src/test/user_model/test_batch_row_column_diff.rs

@@ -0,0 +1,154 @@
+#![allow(clippy::unwrap_used)]
+
+use bitcode::decode;
+
+use crate::{
+    test::util::new_empty_model,
+    user_model::history::{Diff, QueueDiffs},
+    UserModel,
+};
+
+fn last_diff_list(model: &mut UserModel) -> Vec<Diff> {
+    let bytes = model.flush_send_queue();
+    let queue: Vec<QueueDiffs> = decode(&bytes).unwrap();
+    // Get the last operation's diff list
+    queue.last().unwrap().list.clone()
+}
+
+#[test]
+fn diff_invariant_insert_rows() {
+    let base = new_empty_model();
+    let mut model = UserModel::from_model(base);
+
+    assert!(model.insert_rows(0, 5, 3).is_ok());
+
+    let list = last_diff_list(&mut model);
+    assert_eq!(list.len(), 3);
+    for diff in list {
+        match diff {
+            Diff::InsertRow { sheet, row } => {
+                assert_eq!(sheet, 0);
+                assert_eq!(row, 5);
+            }
+            _ => panic!("Unexpected diff variant"),
+        }
+    }
+}
+
+#[test]
+fn diff_invariant_insert_columns() {
+    let base = new_empty_model();
+    let mut model = UserModel::from_model(base);
+
+    assert!(model.insert_columns(0, 2, 4).is_ok());
+    let list = last_diff_list(&mut model);
+    assert_eq!(list.len(), 4);
+    for diff in list {
+        match diff {
+            Diff::InsertColumn { sheet, column } => {
+                assert_eq!(sheet, 0);
+                assert_eq!(column, 2);
+            }
+            _ => panic!("Unexpected diff variant"),
+        }
+    }
+}
+
+#[test]
+fn undo_redo_after_batch_delete() {
+    let base = new_empty_model();
+    let mut model = UserModel::from_model(base);
+
+    // Place values that will shift.
+    model.set_user_input(0, 20, 1, "A").unwrap();
+    model.set_user_input(0, 1, 20, "B").unwrap();
+
+    // Fill the rows we are about to delete so the diff list can be produced
+    for r in 10..15 {
+        model.set_user_input(0, r, 1, "tmp").unwrap();
+    }
+
+    // Delete rows 10..14 and columns 5..8
+    assert!(model.delete_rows(0, 10, 5).is_ok());
+    assert!(model.delete_columns(0, 5, 4).is_ok());
+
+    // Verify shift
+    assert_eq!(model.get_formatted_cell_value(0, 15, 1).unwrap(), "A");
+    assert_eq!(model.get_formatted_cell_value(0, 1, 16).unwrap(), "B");
+
+    // Undo
+    model.undo().unwrap(); // columns
+    model.undo().unwrap(); // rows
+    assert_eq!(model.get_formatted_cell_value(0, 20, 1).unwrap(), "A");
+    assert_eq!(model.get_formatted_cell_value(0, 1, 20).unwrap(), "B");
+
+    // Redo
+    model.redo().unwrap(); // rows
+    model.redo().unwrap(); // columns
+    assert_eq!(model.get_formatted_cell_value(0, 15, 1).unwrap(), "A");
+    assert_eq!(model.get_formatted_cell_value(0, 1, 16).unwrap(), "B");
+}
+
+#[test]
+fn diff_order_delete_rows() {
+    // Verifies that delete diffs are generated bottom-to-top
+    let base = new_empty_model();
+    let mut model = UserModel::from_model(base);
+
+    // Populate rows to delete
+    for r in 5..10 {
+        model.set_user_input(0, r, 1, &r.to_string()).unwrap();
+    }
+
+    assert!(model.delete_rows(0, 5, 5).is_ok());
+
+    let list = last_diff_list(&mut model);
+    assert_eq!(list.len(), 5);
+
+    // Diffs should be in reverse order: 9, 8, 7, 6, 5
+    let mut expected_row = 9;
+    for diff in list {
+        match diff {
+            Diff::DeleteRow { row, .. } => {
+                assert_eq!(row, expected_row);
+                expected_row -= 1;
+            }
+            _ => panic!("Unexpected diff variant"),
+        }
+    }
+}
+
+#[test]
+fn batch_operations_with_formulas() {
+    // Verifies formulas update correctly after batch ops
+    let base = new_empty_model();
+    let mut model = UserModel::from_model(base);
+
+    model.set_user_input(0, 1, 1, "10").unwrap();
+    model.set_user_input(0, 5, 1, "=A1*2").unwrap(); // Will become A3 after insert
+
+    assert!(model.insert_rows(0, 2, 2).is_ok());
+
+    // Formula should now reference A1 (unchanged) but be in row 7
+    assert_eq!(model.get_formatted_cell_value(0, 7, 1).unwrap(), "20");
+    assert_eq!(model.get_cell_content(0, 7, 1).unwrap(), "=A1*2");
+
+    // Undo and verify formula is back at original position
+    model.undo().unwrap();
+    assert_eq!(model.get_formatted_cell_value(0, 5, 1).unwrap(), "20");
+}
+
+#[test]
+fn edge_case_single_operation() {
+    // Single row/column operations should still work correctly
+    let base = new_empty_model();
+    let mut model = UserModel::from_model(base);
+
+    assert!(model.insert_rows(0, 1, 1).is_ok());
+    let list = last_diff_list(&mut model);
+    assert_eq!(list.len(), 1);
+
+    assert!(model.insert_columns(0, 1, 1).is_ok());
+    let list = last_diff_list(&mut model);
+    assert_eq!(list.len(), 1);
+}

base/src/user_model/common.rs

@@ -858,14 +858,23 @@ impl UserModel {
         Ok(())
     }
 
-    /// Inserts several rows at once
+    /// Inserts `row_count` blank rows starting at `row` (both 0-based).
+    ///
+    /// Parameters
+    /// * `sheet` – worksheet index.
+    /// * `row` – first row to insert.
+    /// * `row_count` – number of rows (> 0).
+    ///
+    /// History: the method pushes `row_count` [`crate::user_model::history::Diff::InsertRow`]
+    /// items **all using the same `row` index**.  Replaying those diffs (undo / redo)
+    /// is therefore immune to the row-shifts that happen after each individual
+    /// insertion.
+    ///
+    /// See also [`Model::insert_rows`].
     pub fn insert_rows(&mut self, sheet: u32, row: i32, row_count: i32) -> Result<(), String> {
         let mut diff_list = Vec::new();
-        for r in 0..row_count {
-            diff_list.push(Diff::InsertRow {
-                sheet,
-                row: row + r,
-            });
+        for _ in 0..row_count {
+            diff_list.push(Diff::InsertRow { sheet, row });
         }
         self.push_diff_list(diff_list);
         self.model.insert_rows(sheet, row, row_count)?;
@@ -873,7 +882,18 @@ impl UserModel {
         Ok(())
     }
 
-    /// Inserts several columns at once
+    /// Inserts `column_count` blank columns starting at `column` (0-based).
+    ///
+    /// Parameters
+    /// * `sheet` – worksheet index.
+    /// * `column` – first column to insert.
+    /// * `column_count` – number of columns (> 0).
+    ///
+    /// History: pushes one [`crate::user_model::history::Diff::InsertColumn`]
+    /// per inserted column, all with the same `column` value, preventing index
+    /// drift when the diffs are reapplied.
+    ///
+    /// See also [`Model::insert_columns`].
     pub fn insert_columns(
         &mut self,
         sheet: u32,
@@ -881,11 +901,8 @@ impl UserModel {
         column_count: i32,
     ) -> Result<(), String> {
         let mut diff_list = Vec::new();
-        for c in 0..column_count {
-            diff_list.push(Diff::InsertColumn {
-                sheet,
-                column: column + c,
-            });
+        for _ in 0..column_count {
+            diff_list.push(Diff::InsertColumn { sheet, column });
         }
         self.push_diff_list(diff_list);
         self.model.insert_columns(sheet, column, column_count)?;
@@ -893,12 +910,20 @@ impl UserModel {
         Ok(())
     }
 
-    /// Deletes several rows at once
+    /// Deletes `row_count` rows starting at `row`.
+    ///
+    /// History: a [`crate::user_model::history::Diff::DeleteRow`] is created for
+    /// each row, ordered **bottom → top**.  Undo therefore recreates rows from
+    /// top → bottom and redo removes them bottom → top, avoiding index drift.
+    ///
+    /// See also [`Model::delete_rows`].
     pub fn delete_rows(&mut self, sheet: u32, row: i32, row_count: i32) -> Result<(), String> {
         let diff_list = {
             let worksheet = self.model.workbook.worksheet(sheet)?;
             let mut diff_list = Vec::new();
-            for r in row..row + row_count {
+            // Collect diffs from bottom to top so that `undo` re-inserts rows
+            // in the correct order (top to bottom).
+            for r in (row..row + row_count).rev() {
                 let mut row_data = None;
                 for rd in &worksheet.rows {
                     if rd.r == r {
@@ -927,7 +952,13 @@ impl UserModel {
         Ok(())
     }
 
-    /// Deletes several columns at once
+    /// Deletes `column_count` columns starting at `column`.
+    ///
+    /// History: pushes one [`crate::user_model::history::Diff::DeleteColumn`]
+    /// per column, **right → left**, so replaying the list is always safe with
+    /// respect to index shifts.
+    ///
+    /// See also [`Model::delete_columns`].
     pub fn delete_columns(
         &mut self,
         sheet: u32,
@@ -937,7 +968,8 @@ impl UserModel {
         let diff_list = {
             let worksheet = self.model.workbook.worksheet(sheet)?;
             let mut diff_list = Vec::new();
-            for c in column..column + column_count {
+            // Iterate from right-to-left so `undo` restores from left-to-right.
+            for c in (column..column + column_count).rev() {
                 if !is_valid_column_number(c) {
                     return Err(format!("Column number '{c}' is not valid."));
                 }
@@ -2497,4 +2529,6 @@ mod tests {
             assert_eq!(horizontal(&format!("{a}")), Ok(a));
         }
     }
+
+    // (batch diff tests moved to separate file)
 }

base/src/user_model/mod.rs

@@ -3,7 +3,7 @@
 mod border;
 mod border_utils;
 mod common;
-mod history;
+pub(crate) mod history;
 mod ui;
 
 pub use common::UserModel;