diff --git a/context/test-evidence/command.md b/context/test-evidence/command.md index e6059fc5..97b8ce12 100644 --- a/context/test-evidence/command.md +++ b/context/test-evidence/command.md @@ -4758,3 +4758,44 @@ a time. A first version of this test called `apply_effects` per command; each single-`Move` batch reverses to the same cursor, so the reversed fold was invisible and the test passed against it. One batch is also the shape a `Binding::Sequence` really has. + +## `ui/project.rs` — a formula lost with its target ITEM (improvise-en4s) + +`Dirty::of(ModelChange::CellData)` reports `formulas: true`. That was written as +deliberate over-reporting in the cheap direction — a cell write cannot change a +formula, so the entry cost one redundant `Projection` and bought nothing. +improvise-g3no made `Model::remove_item` drop the formulas targeting the item, +and `effect::RemoveItem` is `CellData`, so a deletion really can take a formula +out of the document. Tightening that arm would leave every connected client's +formula panel showing a formula the document no longer has, **forever**, with +nothing in the stream to correct it — gotcha 35's expensive direction, and the +exact failure that put `formulas` on the `LayoutGeometry` arm in the first place. + +### removing_a_formulas_target_item_empties_the_clients_formula_panel + +The item-level twin of `removing_a_category_empties_the_clients_formula_panel`, +which covers the `LayoutGeometry` arm only. + +Fault injected: `formulas: false` on the `CellData` arm of `Dirty::of`. The test +fails on the final assertion — the client still names `Total = Revenue` after the +document has dropped it. + +**The setup had to be restructured so it could fail there.** Written the obvious +way, mirroring its sibling — connect, then `AddFormula`, then `RemoveItem` — it +does fail under the rival, but at the *setup* assertion "the client must be +holding the formula before it can lose it", because `AddFormula` is **itself** +`CellData`, so with the arm tightened the formula never reaches the client at +all. That answers "did `AddFormula`'s projection arrive", which is the +neighbouring question; the property is about a *deletion*'s projection. So the +formula is added **before** `Sessions::connect` and the client picks it up from +the bootstrap seed, which no `Dirty` gates. Both assertions are then reachable +independently and the rival lands on the one that means something. + +The sibling does not have this problem against its *own* rival: `formulas: false` +on the `LayoutGeometry` arm leaves `AddFormula` delivering normally, so its setup +holds and only its final assertion moves. It is left alone. + +The test also asserts `RemoveItem::kind()` is `Model(ModelChange::CellData)` +before applying it, so reclassifying that effect fails here rather than silently +turning this into a test of the `LayoutGeometry` arm that its sibling already +covers. diff --git a/crates/improvise-command/src/ui/project.rs b/crates/improvise-command/src/ui/project.rs index 0d19ac29..7d33657b 100644 --- a/crates/improvise-command/src/ui/project.rs +++ b/crates/improvise-command/src/ui/project.rs @@ -1124,7 +1124,9 @@ fn fresh_view(app: &App) -> ViewState { mod tests { use super::*; use crate::model::cell::{CellKey, CellValue}; - use crate::ui::effect::{AddFormula, Effect, RemoveCategory, SetAxis, SetCell, ViewTarget}; + use crate::ui::effect::{ + AddFormula, Effect, RemoveCategory, RemoveItem, SetAxis, SetCell, ViewTarget, + }; use crate::workbook::Workbook; use improvise_protocol::{ BasisCache, CategoryTreeCache, DocumentCache, FormulaCache, TileBarCache, ViewListCache, @@ -1696,6 +1698,55 @@ mod tests { ); } + /// The item-level twin of the test above. Evidence: + /// `context/test-evidence/command.md`. + #[test] + fn removing_a_formulas_target_item_empties_the_clients_formula_panel() { + let mut app = app_with_data(); + apply( + &mut app, + vec![Box::new(AddFormula { + raw: "Total = Revenue".to_string(), + target_category: "_Measure".to_string(), + })], + ); + + // The formula predates the session, so the client holds it from the + // BOOTSTRAP SEED and not from a CellData projection (improvise-en4s). + let mut sessions = Sessions::default(); + let (_, snapshot) = sessions.connect(&app, 100, 100); + let mut client = ClientCaches::default(); + client.fold(&snapshot.seed); + assert_eq!( + client.formulas.formulas, + vec!["Total = Revenue".to_string()], + "the client must be holding the formula before it can lose it" + ); + + let effect = RemoveItem { + category: "_Measure".to_string(), + item: "Total".to_string(), + }; + assert_eq!( + effect.kind(), + EffectKind::Model(ModelChange::CellData), + "this test is about the CellData arm; reclassify it and it stops being that" + ); + let dirty = apply(&mut app, vec![Box::new(effect)]); + for (_, projections) in sessions.project_all(&app, dirty) { + client.fold(&projections); + } + assert!( + app.model_state.workbook.model.formulas().is_empty(), + "the document dropped the formula with its target item" + ); + assert_eq!( + client.formulas.formulas, + Vec::::new(), + "the client is still showing a formula the document no longer has" + ); + } + /// Bootstrap is a compaction, not a replay: a late client folding only the seed /// lands where one that followed the whole stream is. #[test]