Merge pull request #157 from fiddlerwoaroof/fix/en4s-formula-panel-item-delete

test(project): guard the formula a target ITEM's deletion takes with it (improvise-en4s)

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.

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::<String>::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]