From 8e816dff1e3e4047e3d15c71e93fb9dfb660734d Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Sun, 1 Nov 2026 14:40:50 -0500 Subject: [PATCH] Engine: Save Model asks before it writes over a Python cell Save Model (new on main) asks before it replaces data in the column it writes to, but it looked for a value or a formula only, so a Python cell whose cached value is empty was taken for an empty cell and its code was replaced without the question. Python code now counts as content there, as it does elsewhere. --- crates/engine/src/cmd/solver/model_range.rs | 8 ++++---- crates/engine/src/tests/python_cells.rs | 22 +++++++++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/solver/model_range.rs b/crates/engine/src/cmd/solver/model_range.rs index 1874a8871..b68b8bfb9 100644 --- a/crates/engine/src/cmd/solver/model_range.rs +++ b/crates/engine/src/cmd/solver/model_range.rs @@ -107,12 +107,12 @@ fn check_column_spills(sheet: &Sheet, column: RangeRef, ours: &[CellRef]) -> Res Ok(()) } -/// Refused, unless `overwrite`, when a cell of the model's column holds a value or a formula -/// (a model saved there before included): Save Model then asks before it replaces data, as -/// Excel does. The message carries [`OVERWRITE_ERROR`], which the dialog turns into its +/// Refused, unless `overwrite`, when a cell of the model's column holds a value, a formula or +/// Python code (a model saved there before included): Save Model then asks before it replaces +/// data, as Excel does. The message carries [`OVERWRITE_ERROR`], which the dialog turns into its /// question. fn check_column_empty(sheet: &Sheet, column: RangeRef, overwrite: bool) -> Result<()> { - let taken = column.iter().find(|c| sheet.cell(*c).is_some_and(|x| !x.value.is_empty() || x.formula.is_some())); + let taken = column.iter().find(|c| sheet.cell(*c).is_some_and(Cell::has_content)); match taken { Some(c) if !overwrite => Err(EngineError::Other(format!( "The range {} {OVERWRITE_ERROR} ({} isn't empty); pass overwrite: true to replace it.", diff --git a/crates/engine/src/tests/python_cells.rs b/crates/engine/src/tests/python_cells.rs index 308f8006c..00e329cf8 100644 --- a/crates/engine/src/tests/python_cells.rs +++ b/crates/engine/src/tests/python_cells.rs @@ -349,3 +349,25 @@ fn unmerging_lets_a_blocked_python_result_spill_again() { s.execute("home.unmergeCells", json!({"range": "A2:B2"})).unwrap(); assert_eq!((super::v(&s, "A1"), super::v(&s, "A2")), (Value::Number(5.0), Value::Number(6.0)), "it spills again"); } + +#[test] +fn save_model_asks_before_it_writes_over_a_python_cell_that_shows_nothing() { + let mut s = s(); + for (cell, input) in [("B1", "=5*A1+4*A2"), ("B2", "=6*A1+4*A2")] { + s.execute("cell.set", json!({"cell": cell, "input": input})).unwrap(); + } + // H2 is a Python cell whose cached value is empty: its code is still content. + { + let d = s.doc_mut().unwrap(); + let wb = Arc::make_mut(&mut d.wb); + let sh = wb.sheet_mut(0).unwrap(); + sh.set_cell( + CellRef::parse("H2").unwrap(), + Cell { value: Value::Empty, formula: None, style: Default::default(), py: Some(Arc::new(PyCode::new("None"))) }, + ); + } + let model = json!({"objective": "B1", "variables": "A1:A2", "constraints": ["B2 <= 24"]}); + let e = s.execute("data.solverSaveModel", json!({"to": "H1", "model": model})).unwrap_err().to_string(); + assert!(e.contains(crate::cmd::analysis::OVERWRITE_ERROR) && e.contains("H2"), "{e}"); + assert_eq!(code(&s, 0, "H2").as_deref(), Some("None"), "nothing was written"); +}