From a2e519efcca9c13d708ce3d4be13da1e47be5720 Mon Sep 17 00:00:00 2001
From: Ed L <el-github@elangley.org>
Date: Tue, 24 Mar 2026 08:06:51 -0700
Subject: [PATCH] refactor: replace CellValue::Empty with Option<CellValue>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously CellValue had three variants: Number, Text, and Empty.
The Empty variant acted as a null sentinel, but the compiler could not
distinguish between "this is a real value" and "this might be empty".
Code that received a CellValue could use it without checking for Empty,
because there was no type-level enforcement.

Now CellValue has only Number and Text. The absence of a value is
represented as None at every API boundary:

  DataStore::get()    → Option<&CellValue>  (was &CellValue / Empty)
  Model::get_cell()   → Option<&CellValue>  (was &CellValue / Empty)
  Model::evaluate()   → Option<CellValue>   (was CellValue::Empty)
  eval_formula()      → Option<CellValue>   (was CellValue::Empty)

Model gains clear_cell() for explicit key removal; ClearCell dispatch
calls it instead of set_cell(key, CellValue::Empty).

The compiler now forces every caller of evaluate/get_cell to handle
the None case explicitly — accidental use of an empty value as if it
were real is caught at compile time rather than silently computing
wrong results.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 src/command/dispatch.rs |   2 +-
 src/import/wizard.rs    |   4 +-
 src/model/cell.rs       |  48 ++++++----------
 src/model/model.rs      | 120 ++++++++++++++++++++--------------------
 src/persistence/mod.rs  |   3 +-
 src/ui/app.rs           |  16 +++---
 src/ui/grid.rs          |   9 +--
 src/view/layout.rs      |   2 +-
 8 files changed, 95 insertions(+), 109 deletions(-)

diff --git a/src/command/dispatch.rs b/src/command/dispatch.rs
index 9debaa7..e8d45c0 100644
--- a/src/command/dispatch.rs
+++ b/src/command/dispatch.rs
@@ -59,7 +59,7 @@ pub fn dispatch(model: &mut Model, cmd: &Command) -> CommandResult {
                 .map(|pair| (pair[0].clone(), pair[1].clone()))
                 .collect();
             let key = CellKey::new(kv);
-            model.set_cell(key, CellValue::Empty);
+            model.clear_cell(&key);
             CommandResult::ok()
         }
 
diff --git a/src/import/wizard.rs b/src/import/wizard.rs
index 478eb2d..1b02fca 100644
--- a/src/import/wizard.rs
+++ b/src/import/wizard.rs
@@ -370,8 +370,8 @@ mod tests {
             ("Measure".to_string(), "revenue".to_string()),
             ("region".to_string(),  "West".to_string()),
         ]);
-        assert_eq!(model.get_cell(&k_east).as_f64(), Some(100.0));
-        assert_eq!(model.get_cell(&k_west).as_f64(), Some(200.0));
+        assert_eq!(model.get_cell(&k_east).and_then(|v| v.as_f64()), Some(100.0));
+        assert_eq!(model.get_cell(&k_west).and_then(|v| v.as_f64()), Some(200.0));
     }
 
     #[test]
diff --git a/src/model/cell.rs b/src/model/cell.rs
index d0b19f3..d49550d 100644
--- a/src/model/cell.rs
+++ b/src/model/cell.rs
@@ -48,20 +48,15 @@ impl std::fmt::Display for CellKey {
 pub enum CellValue {
     Number(f64),
     Text(String),
-    Empty,
 }
 
 impl CellValue {
     pub fn as_f64(&self) -> Option<f64> {
         match self {
             CellValue::Number(n) => Some(*n),
-            _ => None,
+            CellValue::Text(_) => None,
         }
     }
-
-    pub fn is_empty(&self) -> bool {
-        matches!(self, CellValue::Empty)
-    }
 }
 
 impl std::fmt::Display for CellValue {
@@ -75,17 +70,10 @@ impl std::fmt::Display for CellValue {
                 }
             }
             CellValue::Text(s) => write!(f, "{s}"),
-            CellValue::Empty => write!(f, ""),
         }
     }
 }
 
-impl Default for CellValue {
-    fn default() -> Self {
-        CellValue::Empty
-    }
-}
-
 /// Serialized as a list of (key, value) pairs so CellKey doesn't need
 /// to implement the `Serialize`-as-string requirement for JSON object keys.
 #[derive(Debug, Clone, Default)]
@@ -118,15 +106,11 @@ impl DataStore {
     }
 
     pub fn set(&mut self, key: CellKey, value: CellValue) {
-        if value.is_empty() {
-            self.cells.remove(&key);
-        } else {
-            self.cells.insert(key, value);
-        }
+        self.cells.insert(key, value);
     }
 
-    pub fn get(&self, key: &CellKey) -> &CellValue {
-        self.cells.get(key).unwrap_or(&CellValue::Empty)
+    pub fn get(&self, key: &CellKey) -> Option<&CellValue> {
+        self.cells.get(key)
     }
 
     pub fn get_mut(&mut self, key: &CellKey) -> Option<&mut CellValue> {
@@ -261,7 +245,7 @@ mod data_store {
     #[test]
     fn get_missing_returns_empty() {
         let store = DataStore::new();
-        assert_eq!(store.get(&key(&[("Region", "East")])), &CellValue::Empty);
+        assert_eq!(store.get(&key(&[("Region", "East")])), None);
     }
 
     #[test]
@@ -269,7 +253,7 @@ mod data_store {
         let mut store = DataStore::new();
         let k = key(&[("Region", "East"), ("Product", "Shirts")]);
         store.set(k.clone(), CellValue::Number(42.0));
-        assert_eq!(store.get(&k), &CellValue::Number(42.0));
+        assert_eq!(store.get(&k), Some(&CellValue::Number(42.0)));
     }
 
     #[test]
@@ -278,15 +262,15 @@ mod data_store {
         let k = key(&[("Region", "East")]);
         store.set(k.clone(), CellValue::Number(1.0));
         store.set(k.clone(), CellValue::Number(99.0));
-        assert_eq!(store.get(&k), &CellValue::Number(99.0));
+        assert_eq!(store.get(&k), Some(&CellValue::Number(99.0)));
     }
 
     #[test]
-    fn setting_empty_removes_key() {
+    fn remove_evicts_key() {
         let mut store = DataStore::new();
         let k = key(&[("Region", "East")]);
         store.set(k.clone(), CellValue::Number(5.0));
-        store.set(k.clone(), CellValue::Empty);
+        store.remove(&k);
         assert!(store.cells().is_empty());
     }
 
@@ -433,17 +417,17 @@ mod prop_tests {
             let key = CellKey::new(pairs);
             let mut store = DataStore::default();
             store.set(key.clone(), CellValue::Number(val));
-            prop_assert_eq!(store.get(&key), &CellValue::Number(val));
+            prop_assert_eq!(store.get(&key), Some(&CellValue::Number(val)));
         }
 
-        /// Setting Empty after a real value: get returns Empty (key is evicted).
+        /// Removing after a real value: get returns None (key is evicted).
         #[test]
         fn datastore_empty_evicts_key(pairs in pairs_map(), val in finite_f64()) {
             let key = CellKey::new(pairs);
             let mut store = DataStore::default();
             store.set(key.clone(), CellValue::Number(val));
-            store.set(key.clone(), CellValue::Empty);
-            prop_assert_eq!(store.get(&key), &CellValue::Empty);
+            store.remove(&key);
+            prop_assert_eq!(store.get(&key), None);
         }
 
         /// The last write to a key wins.
@@ -457,7 +441,7 @@ mod prop_tests {
             let mut store = DataStore::default();
             store.set(key.clone(), CellValue::Number(v1));
             store.set(key.clone(), CellValue::Number(v2));
-            prop_assert_eq!(store.get(&key), &CellValue::Number(v2));
+            prop_assert_eq!(store.get(&key), Some(&CellValue::Number(v2)));
         }
 
         /// Two keys that differ by one coordinate are fully independent.
@@ -480,9 +464,9 @@ mod prop_tests {
             let mut store = DataStore::default();
             store.set(key1.clone(), CellValue::Number(v1));
             store.set(key2.clone(), CellValue::Number(v2));
-            prop_assert_eq!(store.get(&key1), &CellValue::Number(v1),
+            prop_assert_eq!(store.get(&key1), Some(&CellValue::Number(v1)),
                 "key1 corrupted after writing key2 (diff in {})", changed_cat);
-            prop_assert_eq!(store.get(&key2), &CellValue::Number(v2));
+            prop_assert_eq!(store.get(&key2), Some(&CellValue::Number(v2)));
         }
 
         /// Every cell returned by matching_cells actually satisfies the partial key.
diff --git a/src/model/model.rs b/src/model/model.rs
index 33d00a9..a615e2e 100644
--- a/src/model/model.rs
+++ b/src/model/model.rs
@@ -67,7 +67,11 @@ impl Model {
         self.data.set(key, value);
     }
 
-    pub fn get_cell(&self, key: &CellKey) -> &CellValue {
+    pub fn clear_cell(&mut self, key: &CellKey) {
+        self.data.remove(key);
+    }
+
+    pub fn get_cell(&self, key: &CellKey) -> Option<&CellValue> {
         self.data.get(key)
     }
 
@@ -144,9 +148,9 @@ impl Model {
         self.categories.keys().map(|s| s.as_str()).collect()
     }
 
-    /// Evaluate a computed value at a given key, considering formulas
-    pub fn evaluate(&self, key: &CellKey) -> CellValue {
-        // Check if the last category dimension in the key corresponds to a formula target
+    /// Evaluate a computed value at a given key, considering formulas.
+    /// Returns None when the cell is empty (no stored value, no applicable formula).
+    pub fn evaluate(&self, key: &CellKey) -> Option<CellValue> {
         for formula in &self.formulas {
             if let Some(item_val) = key.get(&formula.target_category) {
                 if item_val == formula.target {
@@ -154,10 +158,10 @@ impl Model {
                 }
             }
         }
-        self.data.get(key).clone()
+        self.data.get(key).cloned()
     }
 
-    fn eval_formula(&self, formula: &Formula, context: &CellKey) -> CellValue {
+    fn eval_formula(&self, formula: &Formula, context: &CellKey) -> Option<CellValue> {
         use crate::formula::{Expr, AggFunc};
 
         // Check WHERE filter first
@@ -166,7 +170,7 @@ impl Model {
                 .map(|v| v == filter.item.as_str())
                 .unwrap_or(false);
             if !matches {
-                return self.data.get(context).clone();
+                return self.data.get(context).cloned();
             }
         }
 
@@ -190,7 +194,7 @@ impl Model {
                 Expr::Ref(name) => {
                     let cat = find_item_category(model, name)?;
                     let new_key = context.clone().with(cat, name);
-                    model.evaluate(&new_key).as_f64()
+                    model.evaluate(&new_key).and_then(|v| v.as_f64())
                 }
                 Expr::BinOp(op, l, r) => {
                     use crate::formula::BinOp;
@@ -272,10 +276,8 @@ impl Model {
             }
         }
 
-        match eval_expr(&formula.expr, context, self, &formula.target_category) {
-            Some(n) => CellValue::Number(n),
-            None => CellValue::Empty,
-        }
+        eval_expr(&formula.expr, context, self, &formula.target_category)
+            .map(CellValue::Number)
     }
 }
 
@@ -332,14 +334,14 @@ mod model_tests {
         m.add_category("Measure").unwrap();
         let k = coord(&[("Region", "East"), ("Measure", "Revenue")]);
         m.set_cell(k.clone(), CellValue::Number(500.0));
-        assert_eq!(m.get_cell(&k), &CellValue::Number(500.0));
+        assert_eq!(m.get_cell(&k), Some(&CellValue::Number(500.0)));
     }
 
     #[test]
     fn get_unset_cell_returns_empty() {
         let m = Model::new("Test");
         let k = coord(&[("Region", "East")]);
-        assert_eq!(m.get_cell(&k), &CellValue::Empty);
+        assert_eq!(m.get_cell(&k), None);
     }
 
     #[test]
@@ -348,7 +350,7 @@ mod model_tests {
         let k = coord(&[("Region", "East")]);
         m.set_cell(k.clone(), CellValue::Number(1.0));
         m.set_cell(k.clone(), CellValue::Number(2.0));
-        assert_eq!(m.get_cell(&k), &CellValue::Number(2.0));
+        assert_eq!(m.get_cell(&k), Some(&CellValue::Number(2.0)));
     }
 
     #[test]
@@ -365,10 +367,10 @@ mod model_tests {
         m.set_cell(k2.clone(), CellValue::Number(200.0));
         m.set_cell(k3.clone(), CellValue::Number(300.0));
         m.set_cell(k4.clone(), CellValue::Number(40.0));
-        assert_eq!(m.get_cell(&k1), &CellValue::Number(100.0));
-        assert_eq!(m.get_cell(&k2), &CellValue::Number(200.0));
-        assert_eq!(m.get_cell(&k3), &CellValue::Number(300.0));
-        assert_eq!(m.get_cell(&k4), &CellValue::Number(40.0));
+        assert_eq!(m.get_cell(&k1), Some(&CellValue::Number(100.0)));
+        assert_eq!(m.get_cell(&k2), Some(&CellValue::Number(200.0)));
+        assert_eq!(m.get_cell(&k3), Some(&CellValue::Number(300.0)));
+        assert_eq!(m.get_cell(&k4), Some(&CellValue::Number(40.0)));
     }
 
     #[test]
@@ -437,7 +439,7 @@ mod model_tests {
         m.create_view("Second");
         let k = coord(&[("Region", "East")]);
         m.set_cell(k.clone(), CellValue::Number(77.0));
-        assert_eq!(m.get_cell(&k), &CellValue::Number(77.0));
+        assert_eq!(m.get_cell(&k), Some(&CellValue::Number(77.0)));
     }
 }
 
@@ -478,7 +480,7 @@ mod formula_tests {
         let mut m = revenue_cost_model();
         m.add_formula(parse_formula("Profit = Revenue - Cost", "Measure").unwrap());
         let k = coord(&[("Measure", "Profit"), ("Region", "East")]);
-        assert_eq!(m.evaluate(&k), CellValue::Number(400.0));
+        assert_eq!(m.evaluate(&k), Some(CellValue::Number(400.0)));
     }
 
     #[test]
@@ -487,8 +489,8 @@ mod formula_tests {
         m.add_formula(parse_formula("Profit = Revenue - Cost", "Measure").unwrap());
         let east = m.evaluate(&coord(&[("Measure", "Profit"), ("Region", "East")]));
         let west = m.evaluate(&coord(&[("Measure", "Profit"), ("Region", "West")]));
-        assert_eq!(east, CellValue::Number(400.0));
-        assert_eq!(west, CellValue::Number(300.0));
+        assert_eq!(east, Some(CellValue::Number(400.0)));
+        assert_eq!(west, Some(CellValue::Number(300.0)));
     }
 
     #[test]
@@ -496,7 +498,7 @@ mod formula_tests {
         let mut m = revenue_cost_model();
         m.add_formula(parse_formula("Tax = Revenue * 0.1", "Measure").unwrap());
         if let Some(cat) = m.category_mut("Measure") { cat.add_item("Tax"); }
-        let val = m.evaluate(&coord(&[("Measure", "Tax"), ("Region", "East")])).as_f64().unwrap();
+        let val = m.evaluate(&coord(&[("Measure", "Tax"), ("Region", "East")])).and_then(|v| v.as_f64()).unwrap();
         assert!(approx_eq(val, 100.0));
     }
 
@@ -509,7 +511,7 @@ mod formula_tests {
             cat.add_item("Profit");
             cat.add_item("Margin");
         }
-        let val = m.evaluate(&coord(&[("Measure", "Margin"), ("Region", "East")])).as_f64().unwrap();
+        let val = m.evaluate(&coord(&[("Measure", "Margin"), ("Region", "East")])).and_then(|v| v.as_f64()).unwrap();
         assert!(approx_eq(val, 0.4));
     }
 
@@ -527,7 +529,7 @@ mod formula_tests {
         m.set_cell(coord(&[("Measure", "Zero"),    ("Region", "East")]), CellValue::Number(0.0));
         m.add_formula(parse_formula("Result = Revenue / Zero", "Measure").unwrap());
         // Division by zero must yield Empty, not 0, so the user sees a blank not a misleading zero.
-        assert_eq!(m.evaluate(&coord(&[("Measure", "Result"), ("Region", "East")])), CellValue::Empty);
+        assert_eq!(m.evaluate(&coord(&[("Measure", "Result"), ("Region", "East")])), None);
     }
 
     #[test]
@@ -536,7 +538,7 @@ mod formula_tests {
         m.add_formula(parse_formula("NegRevenue = -Revenue", "Measure").unwrap());
         if let Some(cat) = m.category_mut("Measure") { cat.add_item("NegRevenue"); }
         let k = coord(&[("Measure", "NegRevenue"), ("Region", "East")]);
-        assert_eq!(m.evaluate(&k), CellValue::Number(-1000.0));
+        assert_eq!(m.evaluate(&k), Some(CellValue::Number(-1000.0)));
     }
 
     #[test]
@@ -549,7 +551,7 @@ mod formula_tests {
         }
         m.set_cell(coord(&[("Measure", "Base")]), CellValue::Number(4.0));
         m.add_formula(parse_formula("Squared = Base ^ 2", "Measure").unwrap());
-        assert_eq!(m.evaluate(&coord(&[("Measure", "Squared")])), CellValue::Number(16.0));
+        assert_eq!(m.evaluate(&coord(&[("Measure", "Squared")])), Some(CellValue::Number(16.0)));
     }
 
     #[test]
@@ -558,7 +560,7 @@ mod formula_tests {
         m.add_formula(parse_formula("Ghost = NoSuchField - Cost", "Measure").unwrap());
         if let Some(cat) = m.category_mut("Measure") { cat.add_item("Ghost"); }
         let k = coord(&[("Measure", "Ghost"), ("Region", "East")]);
-        assert_eq!(m.evaluate(&k), CellValue::Empty);
+        assert_eq!(m.evaluate(&k), None);
     }
 
     #[test]
@@ -566,7 +568,7 @@ mod formula_tests {
         let mut m = revenue_cost_model();
         m.add_formula(parse_formula("EastOnly = Revenue WHERE Region = \"East\"", "Measure").unwrap());
         if let Some(cat) = m.category_mut("Measure") { cat.add_item("EastOnly"); }
-        let val = m.evaluate(&coord(&[("Measure", "EastOnly"), ("Region", "East")])).as_f64().unwrap();
+        let val = m.evaluate(&coord(&[("Measure", "EastOnly"), ("Region", "East")])).and_then(|v| v.as_f64()).unwrap();
         assert!(approx_eq(val, 1000.0));
     }
 
@@ -575,7 +577,7 @@ mod formula_tests {
         let mut m = revenue_cost_model();
         m.add_formula(parse_formula("EastOnly = Revenue WHERE Region = \"East\"", "Measure").unwrap());
         if let Some(cat) = m.category_mut("Measure") { cat.add_item("EastOnly"); }
-        assert_eq!(m.evaluate(&coord(&[("Measure", "EastOnly"), ("Region", "West")])), CellValue::Empty);
+        assert_eq!(m.evaluate(&coord(&[("Measure", "EastOnly"), ("Region", "West")])), None);
     }
 
     #[test]
@@ -585,7 +587,7 @@ mod formula_tests {
         m.add_formula(parse_formula("Profit = Revenue - Cost - 100", "Measure").unwrap());
         assert_eq!(m.formulas.len(), 1);
         let k = coord(&[("Measure", "Profit"), ("Region", "East")]);
-        assert_eq!(m.evaluate(&k), CellValue::Number(300.0));
+        assert_eq!(m.evaluate(&k), Some(CellValue::Number(300.0)));
     }
 
     #[test]
@@ -595,7 +597,7 @@ mod formula_tests {
         m.remove_formula("Profit", "Measure");
         assert!(m.formulas.is_empty());
         let k = coord(&[("Measure", "Profit"), ("Region", "East")]);
-        assert_eq!(m.evaluate(&k), CellValue::Empty);
+        assert_eq!(m.evaluate(&k), None);
     }
 
     #[test]
@@ -603,7 +605,7 @@ mod formula_tests {
         let mut m = revenue_cost_model();
         m.add_formula(parse_formula("Total = SUM(Revenue)", "Measure").unwrap());
         if let Some(cat) = m.category_mut("Measure") { cat.add_item("Total"); }
-        let val = m.evaluate(&coord(&[("Measure", "Total"), ("Region", "East")])).as_f64().unwrap();
+        let val = m.evaluate(&coord(&[("Measure", "Total"), ("Region", "East")])).and_then(|v| v.as_f64()).unwrap();
         // Revenue(East)=1000 only — Cost must not be included
         assert_eq!(val, 1000.0);
     }
@@ -621,7 +623,7 @@ mod formula_tests {
             m.set_cell(coord(&[("Measure", "Sales"), ("Region", region)]), CellValue::Number(100.0));
         }
         m.add_formula(parse_formula("Count = COUNT(Sales)", "Measure").unwrap());
-        let val = m.evaluate(&coord(&[("Measure", "Count"), ("Region", "East")])).as_f64().unwrap();
+        let val = m.evaluate(&coord(&[("Measure", "Count"), ("Region", "East")])).and_then(|v| v.as_f64()).unwrap();
         assert!(val >= 1.0);
     }
 
@@ -635,7 +637,7 @@ mod formula_tests {
         }
         m.set_cell(coord(&[("Measure", "X")]), CellValue::Number(10.0));
         m.add_formula(parse_formula("Result = IF(X > 5, 1, 0)", "Measure").unwrap());
-        assert_eq!(m.evaluate(&coord(&[("Measure", "Result")])), CellValue::Number(1.0));
+        assert_eq!(m.evaluate(&coord(&[("Measure", "Result")])), Some(CellValue::Number(1.0)));
     }
 
     #[test]
@@ -648,7 +650,7 @@ mod formula_tests {
         }
         m.set_cell(coord(&[("Measure", "X")]), CellValue::Number(3.0));
         m.add_formula(parse_formula("Result = IF(X > 5, 1, 0)", "Measure").unwrap());
-        assert_eq!(m.evaluate(&coord(&[("Measure", "Result")])), CellValue::Number(0.0));
+        assert_eq!(m.evaluate(&coord(&[("Measure", "Result")])), Some(CellValue::Number(0.0)));
     }
 
     // ── Bug regression tests ─────────────────────────────────────────────────
@@ -677,7 +679,7 @@ mod formula_tests {
         let key_no_region = coord(&[("Measure", "Profit")]);
         // Expected: Empty (formula should not apply)
         // Bug: returns Number(42) — formula applied because absent category falls through
-        assert_eq!(m.evaluate(&key_no_region), CellValue::Empty);
+        assert_eq!(m.evaluate(&key_no_region), None);
     }
 
     /// Bug: SUM(Revenue) ignores its inner expression and sums all numeric
@@ -705,7 +707,7 @@ mod formula_tests {
         // Bug: returns 150 — inner Ref("Revenue") is ignored, Cost is also summed
         assert_eq!(
             m.evaluate(&coord(&[("Measure", "Total"), ("Region", "East")])),
-            CellValue::Number(100.0),
+            Some(CellValue::Number(100.0)),
         );
     }
 
@@ -744,8 +746,8 @@ mod formula_tests {
 
         // Measure formula → 1, KPI formula → 2
         // Bug: first formula was replaced; {Measure=Profit} evaluates to Empty.
-        assert_eq!(m.evaluate(&coord(&[("Measure", "Profit")])), CellValue::Number(1.0));
-        assert_eq!(m.evaluate(&coord(&[("KPI",     "Profit")])), CellValue::Number(2.0));
+        assert_eq!(m.evaluate(&coord(&[("Measure", "Profit")])), Some(CellValue::Number(1.0)));
+        assert_eq!(m.evaluate(&coord(&[("KPI",     "Profit")])), Some(CellValue::Number(2.0)));
     }
 
     /// Bug: remove_formula matches by target name alone, so removing "Profit"
@@ -768,7 +770,7 @@ mod formula_tests {
         // KPI formula must survive
         // Bug: remove_formula("Profit") wipes both; formulas.len() == 0
         assert_eq!(m.formulas.len(), 1);
-        assert_eq!(m.evaluate(&coord(&[("KPI", "Profit")])), CellValue::Number(2.0));
+        assert_eq!(m.evaluate(&coord(&[("KPI", "Profit")])), Some(CellValue::Number(2.0)));
     }
 }
 
@@ -846,7 +848,7 @@ mod five_category {
     fn all_sixteen_revenue_cells_stored() {
         let m = build_model();
         let count = DATA.iter()
-            .filter(|&&(r, p, c, t, _, _)| !m.get_cell(&coord(r, p, c, t, "Revenue")).is_empty())
+            .filter(|&&(r, p, c, t, _, _)| !m.get_cell(&coord(r, p, c, t, "Revenue")).is_none())
             .count();
         assert_eq!(count, 16);
     }
@@ -855,7 +857,7 @@ mod five_category {
     fn all_sixteen_cost_cells_stored() {
         let m = build_model();
         let count = DATA.iter()
-            .filter(|&&(r, p, c, t, _, _)| !m.get_cell(&coord(r, p, c, t, "Cost")).is_empty())
+            .filter(|&&(r, p, c, t, _, _)| !m.get_cell(&coord(r, p, c, t, "Cost")).is_none())
             .count();
         assert_eq!(count, 16);
     }
@@ -863,8 +865,8 @@ mod five_category {
     #[test]
     fn spot_check_raw_revenue() {
         let m = build_model();
-        assert_eq!(m.get_cell(&coord("East", "Shirts", "Online", "Q1", "Revenue")), &CellValue::Number(1_000.0));
-        assert_eq!(m.get_cell(&coord("West", "Pants",  "Retail", "Q2", "Revenue")), &CellValue::Number(280.0));
+        assert_eq!(m.get_cell(&coord("East", "Shirts", "Online", "Q1", "Revenue")), Some(&CellValue::Number(1_000.0)));
+        assert_eq!(m.get_cell(&coord("West", "Pants",  "Retail", "Q2", "Revenue")), Some(&CellValue::Number(280.0)));
     }
 
     #[test]
@@ -881,7 +883,7 @@ mod five_category {
         for &(region, product, channel, time, rev, cost) in DATA {
             let expected = rev - cost;
             let actual = m.evaluate(&coord(region, product, channel, time, "Profit"))
-                          .as_f64()
+                          .and_then(|v| v.as_f64())
                           .unwrap_or_else(|| panic!("Profit empty at {region}/{product}/{channel}/{time}"));
             assert!(approx(actual, expected),
                 "Profit at {region}/{product}/{channel}/{time}: expected {expected}, got {actual}");
@@ -894,7 +896,7 @@ mod five_category {
         for &(region, product, channel, time, rev, cost) in DATA {
             let expected = (rev - cost) / rev;
             let actual = m.evaluate(&coord(region, product, channel, time, "Margin"))
-                          .as_f64()
+                          .and_then(|v| v.as_f64())
                           .unwrap_or_else(|| panic!("Margin empty at {region}/{product}/{channel}/{time}"));
             assert!(approx(actual, expected),
                 "Margin at {region}/{product}/{channel}/{time}: expected {expected:.4}, got {actual:.4}");
@@ -904,7 +906,7 @@ mod five_category {
     #[test]
     fn chained_formula_profit_feeds_margin() {
         let m = build_model();
-        let margin = m.evaluate(&coord("East", "Shirts", "Online", "Q1", "Margin")).as_f64().unwrap();
+        let margin = m.evaluate(&coord("East", "Shirts", "Online", "Q1", "Margin")).and_then(|v| v.as_f64()).unwrap();
         assert!(approx(margin, 0.4), "expected 0.4, got {margin}");
     }
 
@@ -912,9 +914,9 @@ mod five_category {
     fn update_revenue_updates_profit_and_margin() {
         let mut m = build_model();
         m.set_cell(coord("East", "Shirts", "Online", "Q1", "Revenue"), CellValue::Number(1_500.0));
-        let profit = m.evaluate(&coord("East", "Shirts", "Online", "Q1", "Profit")).as_f64().unwrap();
+        let profit = m.evaluate(&coord("East", "Shirts", "Online", "Q1", "Profit")).and_then(|v| v.as_f64()).unwrap();
         assert!(approx(profit, 900.0), "expected 900, got {profit}");
-        let margin = m.evaluate(&coord("East", "Shirts", "Online", "Q1", "Margin")).as_f64().unwrap();
+        let margin = m.evaluate(&coord("East", "Shirts", "Online", "Q1", "Margin")).and_then(|v| v.as_f64()).unwrap();
         assert!(approx(margin, 0.6), "expected 0.6, got {margin}");
     }
 
@@ -985,7 +987,7 @@ mod five_category {
             v.set_axis("Time",    Axis::Column);
             v.set_axis("Measure", Axis::Page);
         }
-        assert_eq!(m.get_cell(&coord("East", "Shirts", "Online", "Q1", "Revenue")), &CellValue::Number(1_000.0));
+        assert_eq!(m.get_cell(&coord("East", "Shirts", "Online", "Q1", "Revenue")), Some(&CellValue::Number(1_000.0)));
     }
 
     #[test]
@@ -1053,7 +1055,7 @@ mod prop_tests {
             }
             let key = CellKey::new(vec![("Cat".into(), item_a.clone())]);
             m.set_cell(key.clone(), CellValue::Number(val));
-            prop_assert_eq!(m.evaluate(&key), CellValue::Number(val));
+            prop_assert_eq!(m.evaluate(&key), Some(CellValue::Number(val)));
         }
 
         /// Writing to cell A does not change cell B when the keys differ.
@@ -1075,8 +1077,8 @@ mod prop_tests {
             let key_b = CellKey::new(vec![("Cat".into(), item_b)]);
             m.set_cell(key_a.clone(), CellValue::Number(val_a));
             m.set_cell(key_b.clone(), CellValue::Number(val_b));
-            prop_assert_eq!(m.evaluate(&key_a), CellValue::Number(val_a));
-            prop_assert_eq!(m.evaluate(&key_b), CellValue::Number(val_b));
+            prop_assert_eq!(m.evaluate(&key_a), Some(CellValue::Number(val_a)));
+            prop_assert_eq!(m.evaluate(&key_b), Some(CellValue::Number(val_b)));
         }
 
         /// Adding a category does not overwrite previously stored cell values.
@@ -1091,7 +1093,7 @@ mod prop_tests {
             let key = CellKey::new(vec![("Measure".into(), "Revenue".into())]);
             m.set_cell(key.clone(), CellValue::Number(val));
             let _ = m.add_category(&new_cat); // may succeed or hit the 12-cat limit
-            prop_assert_eq!(m.evaluate(&key), CellValue::Number(val));
+            prop_assert_eq!(m.evaluate(&key), Some(CellValue::Number(val)));
         }
 
         /// evaluate() is deterministic: calling it twice on the same key and model
@@ -1145,7 +1147,7 @@ mod prop_tests {
             m.set_cell(cost_key, CellValue::Number(cost));
             m.add_formula(parse_formula("Profit = Revenue - Cost", "Measure").unwrap());
             let expected = rev - cost;
-            prop_assert_eq!(m.evaluate(&profit_key), CellValue::Number(expected));
+            prop_assert_eq!(m.evaluate(&profit_key), Some(CellValue::Number(expected)));
         }
 
         /// Removing a formula restores the raw stored value (or Empty).
@@ -1172,10 +1174,10 @@ mod prop_tests {
             m.add_formula(parse_formula("Profit = Revenue - Cost", "Measure").unwrap());
             // Formula active — result is rev - cost
             let with_formula = m.evaluate(&profit_key);
-            prop_assert_eq!(with_formula, CellValue::Number(rev - cost));
+            prop_assert_eq!(with_formula, Some(CellValue::Number(rev - cost)));
             // Remove formula — cell has no raw value, so Empty
             m.remove_formula("Profit", "Measure");
-            prop_assert_eq!(m.evaluate(&profit_key), CellValue::Empty);
+            prop_assert_eq!(m.evaluate(&profit_key), None);
         }
     }
 }
diff --git a/src/persistence/mod.rs b/src/persistence/mod.rs
index d0f4ff7..f25039c 100644
--- a/src/persistence/mod.rs
+++ b/src/persistence/mod.rs
@@ -100,7 +100,8 @@ pub fn export_csv(model: &Model, view_name: &str, path: &Path) -> Result<()> {
         let row_values: Vec<String> = col_indices.iter().map(|&col_opt| {
             match (row_opt, col_opt) {
                 (Some(ri), Some(ci)) => layout.cell_key(ri, ci)
-                    .map(|key| model.evaluate(&key).to_string())
+                    .and_then(|key| model.evaluate(&key))
+                    .map(|v| v.to_string())
                     .unwrap_or_default(),
                 _ => String::new(),
             }
diff --git a/src/ui/app.rs b/src/ui/app.rs
index 6f8fbe3..1185973 100644
--- a/src/ui/app.rs
+++ b/src/ui/app.rs
@@ -225,7 +225,8 @@ impl App {
             | (KeyCode::Char('i'), KeyModifiers::NONE)
             | (KeyCode::Char('a'), KeyModifiers::NONE) => {
                 let current = self.selected_cell_key()
-                    .map(|k| self.model.get_cell(&k).to_string())
+                    .and_then(|k| self.model.get_cell(&k).cloned())
+                    .map(|v| v.to_string())
                     .unwrap_or_default();
                 self.mode = AppMode::Editing { buffer: current };
             }
@@ -254,7 +255,6 @@ impl App {
                         let cmd = match &value {
                             CellValue::Number(n) => Command::SetCell { coords, value: crate::command::types::CellValueArg::Number { number: *n } },
                             CellValue::Text(t) => Command::SetCell { coords, value: crate::command::types::CellValueArg::Text { text: t.clone() } },
-                            CellValue::Empty => Command::ClearCell { coords },
                         };
                         command::dispatch(&mut self.model, &cmd);
                         self.dirty = true;
@@ -312,8 +312,7 @@ impl App {
             // yy = yank current cell
             ('y', KeyCode::Char('y')) => {
                 if let Some(key) = self.selected_cell_key() {
-                    let val = self.model.evaluate(&key);
-                    self.yanked = Some(val);
+                    self.yanked = self.model.evaluate(&key);
                     self.status_msg = "Yanked".to_string();
                 }
             }
@@ -996,11 +995,10 @@ impl App {
                 Some(k) => k,
                 None => return false,
             };
-            let val = self.model.evaluate(&key);
-            let s = match &val {
-                CellValue::Number(n) => format!("{n}"),
-                CellValue::Text(t) => t.clone(),
-                CellValue::Empty => String::new(),
+            let s = match self.model.evaluate(&key) {
+                Some(CellValue::Number(n)) => format!("{n}"),
+                Some(CellValue::Text(t)) => t,
+                None => String::new(),
             };
             s.to_lowercase().contains(&query)
         }).collect();
diff --git a/src/ui/grid.rs b/src/ui/grid.rs
index adf9bc9..674dfb2 100644
--- a/src/ui/grid.rs
+++ b/src/ui/grid.rs
@@ -97,7 +97,9 @@ impl<'a> GridWidget<'a> {
                 };
                 let value = self.model.evaluate(&key);
 
-                let cell_str = format_value(&value, fmt_comma, fmt_decimals);
+                let cell_str = value.as_ref()
+                    .map(|v| format_value(v, fmt_comma, fmt_decimals))
+                    .unwrap_or_default();
                 let is_selected = ri == sel_row && ci == sel_col;
                 let is_search_match = !self.search_query.is_empty()
                     && cell_str.to_lowercase().contains(&self.search_query.to_lowercase());
@@ -106,7 +108,7 @@ impl<'a> GridWidget<'a> {
                     Style::default().fg(Color::Black).bg(Color::Cyan).add_modifier(Modifier::BOLD)
                 } else if is_search_match {
                     Style::default().fg(Color::Black).bg(Color::Yellow)
-                } else if matches!(value, CellValue::Empty) {
+                } else if value.is_none() {
                     Style::default().fg(Color::DarkGray)
                 } else {
                     Style::default()
@@ -149,7 +151,7 @@ impl<'a> GridWidget<'a> {
                     if x >= area.x + area.width { break; }
                     let total: f64 = (0..layout.row_count())
                         .filter_map(|ri| layout.cell_key(ri, ci))
-                        .map(|key| self.model.evaluate(&key).as_f64().unwrap_or(0.0))
+                        .map(|key| self.model.evaluate(&key).and_then(|v| v.as_f64()).unwrap_or(0.0))
                         .sum();
                     let total_str = format_f64(total, fmt_comma, fmt_decimals);
                     buf.set_string(x, y,
@@ -202,7 +204,6 @@ fn format_value(v: &CellValue, comma: bool, decimals: u8) -> String {
     match v {
         CellValue::Number(n) => format_f64(*n, comma, decimals),
         CellValue::Text(s) => s.clone(),
-        CellValue::Empty => String::new(),
     }
 }
 
diff --git a/src/view/layout.rs b/src/view/layout.rs
index 18bbe4c..3dd5efe 100644
--- a/src/view/layout.rs
+++ b/src/view/layout.rs
@@ -162,7 +162,7 @@ mod tests {
         let layout = GridLayout::new(&m, m.active_view().unwrap());
         // Clothing = row 1, Feb = col 1
         let key = layout.cell_key(1, 1).unwrap();
-        assert_eq!(m.evaluate(&key), CellValue::Number(42.0));
+        assert_eq!(m.evaluate(&key), Some(CellValue::Number(42.0)));
     }
 
     #[test]