diff --git a/crates/engine/src/cmd/whatif/scenarios.rs b/crates/engine/src/cmd/whatif/scenarios.rs index cd779b2fb..9ebb1efb5 100644 --- a/crates/engine/src/cmd/whatif/scenarios.rs +++ b/crates/engine/src/cmd/whatif/scenarios.rs @@ -228,7 +228,8 @@ pub(super) fn scenario_summary(s: &mut Session, p: &Json) -> Result { } // 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 = changing.iter().map(|c| sh.value(*c)).collect(); diff --git a/crates/engine/src/cmd/whatif/tests.rs b/crates/engine/src/cmd/whatif/tests.rs index 1c7f8c6f5..bf5118873 100644 --- a/crates/engine/src/cmd/whatif/tests.rs +++ b/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}"); +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 8c79c9b44..a2b1a9358 100644 --- a/crates/engine/src/lib.rs +++ b/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::())); sum.saturating_add(std::mem::size_of::() + c.author.len() + c.text.len() + replies + dates) }); sheet.hyperlinks.values().fold(comments, |sum, h| { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 794134538..0003a048e 100644 --- a/crates/engine/src/tests.rs +++ b/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)>()); +}