address review: count result rows in the scenario summary limit and reply/date vector storage in the history estimate

crates/engine/src/cmd/whatif/scenarios.rs

@@ -228,7 +228,8 @@ pub(super) fn scenario_summary(s: &mut Session, p: &Json) -> Result<Json> {
     }
     // The summary has a cell for every changing cell in every scenario, filled from the current
     // value where a scenario doesn't set it, so small disjoint scenarios multiply.
-    if changing.len().saturating_mul(per.len()) > MAX_SUMMARY_CELLS {
+    // Each result cell is a row of the summary too, with a cell per scenario.
+    if changing.len().saturating_add(results.len()).saturating_mul(per.len()) > MAX_SUMMARY_CELLS {
         return Err(bad(CMD, "the scenarios together change too many cells for a summary"));
     }
     let current: Vec<Value> = changing.iter().map(|c| sh.value(*c)).collect();

crates/engine/src/cmd/whatif/tests.rs

@@ -308,3 +308,14 @@ fn scenario_summary_refuses_a_matrix_over_its_cell_limit() {
     }
     assert!(s.execute("data.scenarioSummary", json!({})).is_err());
 }
+
+#[test]
+fn scenario_summary_counts_result_rows_in_its_cell_limit() {
+    let mut s = s();
+    // 900 scenarios of one distinct cell each are 810,000 cells; 255 result rows take it past the limit.
+    for i in 0..900 {
+        s.execute("data.scenarioAdd", json!({"name": format!("S{i}"), "changing": format!("A{}", i + 1)})).unwrap();
+    }
+    let err = s.execute("data.scenarioSummary", json!({"resultCells": "C1:C255"})).unwrap_err().to_string();
+    assert!(err.contains("too many cells"), "{err}");
+}

crates/engine/src/lib.rs

@@ -241,8 +241,11 @@ const UNDO_BYTES_LIMIT: usize = 128 * 1024 * 1024;
 /// strings (an edit deep-copies them), so they count against the history budget with the cells.
 fn sheet_text_bytes(sheet: &gridcraft_model::Sheet) -> usize {
     let comments = sheet.comments.values().fold(0usize, |sum, c| {
+        // The vectors' own storage counts too: an empty reply still takes an element.
         let replies = c.replies.iter().fold(0usize, |n, (a, t)| n.saturating_add(a.len()).saturating_add(t.len()));
+        let replies = replies.saturating_add(c.replies.capacity().saturating_mul(std::mem::size_of::<(String, String)>()));
         let dates = c.dates.iter().fold(0usize, |n, d| n.saturating_add(d.len()));
+        let dates = dates.saturating_add(c.dates.capacity().saturating_mul(std::mem::size_of::<String>()));
         sum.saturating_add(std::mem::size_of::<gridcraft_model::Comment>() + c.author.len() + c.text.len() + replies + dates)
     });
     sheet.hyperlinks.values().fold(comments, |sum, h| {

crates/engine/src/tests.rs

@@ -5583,3 +5583,15 @@ fn history_charges_a_changed_sheets_comment_text() {
     // The older snapshot owns the comment text now that the sheet changed under it.
     assert!(crate::history_retained_bytes(&older, &newer) >= 30_000);
 }
+
+#[test]
+fn history_charges_the_storage_of_empty_comment_replies() {
+    let mut s = s();
+    s.execute("review.newComment", json!({"cell": "A1", "text": "c", "author": "Ann"})).unwrap();
+    for _ in 0..500 {
+        s.execute("review.replyComment", json!({"cell": "A1", "text": "", "author": ""})).unwrap();
+    }
+    let sheet = s.doc().unwrap().wb.sheet(0).unwrap().clone();
+    // 500 replies of no text are still 500 elements.
+    assert!(crate::sheet_text_bytes(&sheet) >= 500 * std::mem::size_of::<(String, String)>());
+}