feat(core): CellKey is Ord and DataStore can log its cell changes (improvise-5r8c)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze

crates/improvise-core/src/model/cell.rs

@@ -7,7 +7,7 @@ use super::symbol::{Symbol, SymbolTable};
 
 /// A cell key: (category, item) pairs, always sorted by category name. The
 /// constructors are the only way to build one — context/code/core.md §1.
-#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub struct CellKey(pub Vec<(String, String)>);
 
 impl CellKey {
@@ -194,6 +194,15 @@ impl RecordEntry {
     }
 }
 
+/// One cell's move in the store: `old` is `None` for a new cell, `new` is
+/// `None` for a removal. ADR 009 clause 3; `context/code/improvise-incremental.md`.
+#[derive(Debug, Clone, PartialEq)]
+pub struct CellChange {
+    pub key: CellKey,
+    pub old: Option<CellValue>,
+    pub new: Option<CellValue>,
+}
+
 #[cfg(test)]
 thread_local! {
     /// How many lookups fell through to set narrowing, for the exact-path test.
@@ -215,6 +224,9 @@ pub struct DataStore {
     /// How many stored cells have each number of coordinates; its largest key
     /// says when a partial can match only one exact cell.
     arities: std::collections::BTreeMap<usize, usize>,
+    /// The changes since `record_changes`, or `None` while nobody listens, so an
+    /// unwatched store keeps no log.
+    changes: Option<Vec<CellChange>>,
 }
 
 /// Resolved through the symbol table and sorted wherever the storage is a
@@ -227,6 +239,7 @@ impl std::fmt::Debug for DataStore {
             index,
             records,
             arities: _,
+            changes: _,
         } = self;
         let cells: Vec<_> = cells
             .iter()
@@ -302,7 +315,19 @@ impl DataStore {
             self.index.entry(*pair).or_default().insert(ikey.clone());
         }
         let width = ikey.0.len();
-        let is_new_cell = self.cells.insert(ikey, value).is_none();
+        let previous = match &mut self.changes {
+            Some(log) => {
+                let previous = self.cells.insert(ikey, value.clone());
+                log.push(CellChange {
+                    key,
+                    old: previous.clone(),
+                    new: Some(value),
+                });
+                previous
+            }
+            None => self.cells.insert(ikey, value),
+        };
+        let is_new_cell = previous.is_none();
         if is_new_cell {
             *self.arities.entry(width).or_default() += 1;
         }
@@ -316,6 +341,24 @@ impl DataStore {
         entry.cells += usize::from(is_new_cell);
     }
 
+    /// Start logging every `set` and `remove` as a [`CellChange`], from empty.
+    pub fn record_changes(&mut self) {
+        self.changes = Some(Vec::new());
+    }
+
+    /// Stop logging and drop what was logged.
+    pub fn stop_recording_changes(&mut self) {
+        self.changes = None;
+    }
+
+    /// The changes since the last drain, oldest first. Logging goes on.
+    pub fn drain_changes(&mut self) -> Vec<CellChange> {
+        self.changes
+            .as_mut()
+            .map(std::mem::take)
+            .unwrap_or_default()
+    }
+
     pub fn get(&self, key: &CellKey) -> Option<&CellValue> {
         let ikey = self.lookup_key(key)?;
         self.cells.get(&ikey)
@@ -340,7 +383,14 @@ impl DataStore {
         let Some(ikey) = self.lookup_key(key) else {
             return;
         };
-        if self.cells.shift_remove(&ikey).is_some() {
+        if let Some(old) = self.cells.shift_remove(&ikey) {
+            if let Some(log) = &mut self.changes {
+                log.push(CellChange {
+                    key: key.clone(),
+                    old: Some(old),
+                    new: None,
+                });
+            }
             if let Some(n) = self.arities.get_mut(&ikey.0.len()) {
                 *n -= 1;
                 if *n == 0 {
@@ -714,6 +764,17 @@ mod cell_key {
         assert!(!k.matches_partial(&partial));
     }
 
+    #[test]
+    fn keys_order_by_their_sorted_coordinates() {
+        let a = key(&[("A", "z")]);
+        let b = key(&[("A", "z"), ("B", "a")]);
+        let c = key(&[("A", "z"), ("B", "b")]);
+        let d = key(&[("C", "a")]);
+        let mut keys = vec![d.clone(), c.clone(), a.clone(), b.clone()];
+        keys.sort();
+        assert_eq!(keys, vec![a, b, c, d]);
+    }
+
     #[test]
     fn display_format() {
         let k = key(&[("Region", "East")]);
@@ -723,7 +784,7 @@ mod cell_key {
 
 #[cfg(test)]
 mod data_store {
-    use super::{CellKey, CellValue, DataStore};
+    use super::{CellChange, CellKey, CellValue, DataStore};
 
     fn key(pairs: &[(&str, &str)]) -> CellKey {
         CellKey::new(
@@ -757,6 +818,48 @@ mod data_store {
         assert_eq!(store.get(&k), Some(&CellValue::Number(99.0)));
     }
 
+    #[test]
+    fn a_store_reports_no_changes_until_asked_to_record_them() {
+        let mut store = DataStore::new();
+        store.set(key(&[("Region", "East")]), CellValue::Number(1.0));
+        assert!(store.drain_changes().is_empty());
+    }
+
+    #[test]
+    fn a_recording_store_reports_a_new_cell_an_overwrite_and_a_removal() {
+        let mut store = DataStore::new();
+        let k = key(&[("Region", "East")]);
+        store.record_changes();
+        store.set(k.clone(), CellValue::Number(1.0));
+        store.set(k.clone(), CellValue::Number(2.0));
+        store.remove(&k);
+        store.remove(&k);
+        let change = |old: Option<f64>, new: Option<f64>| CellChange {
+            key: k.clone(),
+            old: old.map(CellValue::Number),
+            new: new.map(CellValue::Number),
+        };
+        assert_eq!(
+            store.drain_changes(),
+            vec![
+                change(None, Some(1.0)),
+                change(Some(1.0), Some(2.0)),
+                change(Some(2.0), None)
+            ],
+            "removing an absent cell reports nothing"
+        );
+        assert!(store.drain_changes().is_empty(), "a drain empties the log");
+    }
+
+    #[test]
+    fn a_store_that_stopped_recording_reports_nothing_more() {
+        let mut store = DataStore::new();
+        store.record_changes();
+        store.stop_recording_changes();
+        store.set(key(&[("Region", "East")]), CellValue::Number(1.0));
+        assert!(store.drain_changes().is_empty());
+    }
+
     #[test]
     fn remove_evicts_key() {
         let mut store = DataStore::new();

crates/improvise-incremental/Cargo.toml

@@ -6,9 +6,10 @@ license = "Apache-2.0"
 description = "The seam between improvise-core and the incremental circuit: the Derived trait, the Eager oracle and the differential harness (ADR 009 clause 2)."
 
 # ADR 009 clause 2: depends on improvise-core, never the other way round. The
-# `zset` edge arrives with the first backend that uses it (step 3), not before.
+# `zset` edge is the cell delta of step 3 (improvise-5r8c).
 [dependencies]
 improvise-core = { path = "../improvise-core" }
+zset = { path = "../zset" }
 
 [dev-dependencies]
 improvise-io = { path = "../improvise-io" }

crates/improvise-incremental/src/delta.rs

@@ -0,0 +1,261 @@
+//! The cell delta: what a write changed, as a Z-set (ADR 009 clause 3).
+//! Reasoning: `context/code/improvise-incremental.md`.
+
+use std::cmp::Ordering;
+
+use improvise_core::model::cell::{CellChange, CellKey, CellValue};
+use improvise_core::model::record::RecordHash;
+use zset::ZSet;
+
+/// A cell value that compares by the float rules, with every NaN one value: `-0.0`
+/// equals `+0.0`. Built only by [`DeltaValue::new`], which makes both true of the
+/// inner value, so the derived-looking order below is a total order.
+#[derive(Debug, Clone)]
+pub struct DeltaValue(CellValue);
+
+impl DeltaValue {
+    pub fn new(value: &CellValue) -> Self {
+        DeltaValue(match value {
+            CellValue::Number(n) if n.is_nan() => CellValue::Number(f64::NAN),
+            CellValue::Number(n) if *n == 0.0 => CellValue::Number(0.0),
+            other => other.clone(),
+        })
+    }
+
+    pub fn value(&self) -> &CellValue {
+        &self.0
+    }
+}
+
+impl Ord for DeltaValue {
+    fn cmp(&self, other: &Self) -> Ordering {
+        match (&self.0, &other.0) {
+            (CellValue::Number(a), CellValue::Number(b)) => a.total_cmp(b),
+            (CellValue::Text(a), CellValue::Text(b)) => a.cmp(b),
+            (CellValue::Error(a), CellValue::Error(b)) => a.cmp(b),
+            (a, b) => rank(a).cmp(&rank(b)),
+        }
+    }
+}
+
+fn rank(value: &CellValue) -> u8 {
+    match value {
+        CellValue::Number(_) => 0,
+        CellValue::Text(_) => 1,
+        CellValue::Error(_) => 2,
+    }
+}
+
+impl PartialOrd for DeltaValue {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl PartialEq for DeltaValue {
+    fn eq(&self, other: &Self) -> bool {
+        self.cmp(other) == Ordering::Equal
+    }
+}
+
+impl Eq for DeltaValue {}
+
+/// What the store's cells changed by: `+(k, v)` new, `-(k, v0) + (k, v1)` overwritten,
+/// `-(k, v0)` removed, and nothing for an overwrite that changes no value.
+pub type CellDelta = ZSet<(CellKey, DeltaValue)>;
+
+/// The delta of `changes`, in order. Changes that cancel leave nothing.
+pub fn cell_delta<'a>(changes: impl IntoIterator<Item = &'a CellChange>) -> CellDelta {
+    let mut delta = CellDelta::new();
+    for change in changes {
+        if let Some(old) = &change.old {
+            delta.insert((change.key.clone(), DeltaValue::new(old)), -1);
+        }
+        if let Some(new) = &change.new {
+            delta.insert((change.key.clone(), DeltaValue::new(new)), 1);
+        }
+    }
+    delta
+}
+
+/// The records `delta` touches, ascending: the record tier of ADR 001's basis.
+pub fn moved_records(delta: &CellDelta) -> Vec<RecordHash> {
+    let mut records: Vec<RecordHash> = delta
+        .iter()
+        .map(|((key, _), _)| RecordHash::of(key))
+        .collect();
+    records.sort_unstable();
+    records.dedup();
+    records
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn key(region: &str, measure: &str) -> CellKey {
+        CellKey::new(vec![
+            ("Region".into(), region.into()),
+            ("_Measure".into(), measure.into()),
+        ])
+    }
+
+    fn change(k: &CellKey, old: Option<CellValue>, new: Option<CellValue>) -> CellChange {
+        CellChange {
+            key: k.clone(),
+            old,
+            new,
+        }
+    }
+
+    fn num(n: f64) -> CellValue {
+        CellValue::Number(n)
+    }
+
+    fn delta_of(old: Option<CellValue>, new: Option<CellValue>) -> CellDelta {
+        cell_delta(&[change(&key("East", "Revenue"), old, new)])
+    }
+
+    #[test]
+    fn a_new_cell_an_overwrite_and_a_removal_have_the_clause_3_shapes() {
+        let k = key("East", "Revenue");
+        let at = |v: f64| (k.clone(), DeltaValue::new(&num(v)));
+        assert_eq!(delta_of(None, Some(num(1.0))), ZSet::singleton(at(1.0), 1));
+        assert_eq!(
+            delta_of(Some(num(1.0)), Some(num(2.0))),
+            ZSet::from_iter([(at(1.0), -1), (at(2.0), 1)])
+        );
+        assert_eq!(delta_of(Some(num(1.0)), None), ZSet::singleton(at(1.0), -1));
+    }
+
+    #[test]
+    fn overwriting_a_value_with_one_that_compares_equal_is_the_zero_delta() {
+        let nans = [
+            f64::NAN,
+            -f64::NAN,
+            f64::from_bits(0x7ff0_0000_0000_0001),
+            f64::from_bits(0xfff8_dead_beef_0001),
+        ];
+        for a in nans {
+            for b in nans {
+                assert!(
+                    delta_of(Some(num(a)), Some(num(b))).is_zero(),
+                    "NaN {:#x} over NaN {:#x}",
+                    a.to_bits(),
+                    b.to_bits()
+                );
+            }
+        }
+        assert!(delta_of(Some(num(0.0)), Some(num(-0.0))).is_zero());
+        assert!(delta_of(Some(num(-0.0)), Some(num(0.0))).is_zero());
+        let text = || Some(CellValue::Text("x".into()));
+        assert!(delta_of(text(), text()).is_zero());
+    }
+
+    #[test]
+    fn values_that_differ_by_the_float_rules_stay_different() {
+        let pairs = [
+            (num(1.0), num(1.0000000000000002)),
+            (num(f64::INFINITY), num(f64::NEG_INFINITY)),
+            (num(f64::NAN), num(0.0)),
+            (num(0.0), CellValue::Text("0".into())),
+            (CellValue::Text("a".into()), CellValue::Error("a".into())),
+        ];
+        for (a, b) in pairs {
+            assert!(
+                !delta_of(Some(a.clone()), Some(b.clone())).is_zero(),
+                "{a:?} over {b:?}"
+            );
+        }
+    }
+
+    #[test]
+    fn the_order_is_total_and_agrees_with_equality() {
+        let values = [
+            num(f64::NEG_INFINITY),
+            num(-1.0),
+            num(-0.0),
+            num(0.0),
+            num(1.0),
+            num(f64::INFINITY),
+            num(f64::NAN),
+            num(-f64::NAN),
+            CellValue::Text(String::new()),
+            CellValue::Text("a".into()),
+            CellValue::Error("a".into()),
+        ];
+        let wrapped: Vec<DeltaValue> = values.iter().map(DeltaValue::new).collect();
+        for a in &wrapped {
+            for b in &wrapped {
+                assert_eq!(a.cmp(b), b.cmp(a).reverse());
+                assert_eq!(a == b, a.cmp(b) == Ordering::Equal);
+                for c in &wrapped {
+                    if a <= b && b <= c {
+                        assert!(a <= c, "{a:?} {b:?} {c:?}");
+                    }
+                }
+            }
+        }
+        assert_eq!(wrapped[2], wrapped[3], "-0.0 is +0.0");
+        assert_eq!(wrapped[6], wrapped[7], "every NaN is one value");
+    }
+
+    #[test]
+    fn numbers_order_by_total_cmp_with_the_canonical_nan_last() {
+        let ascending = [f64::NEG_INFINITY, -1.5, -0.0, 2.0, f64::INFINITY, -f64::NAN];
+        let mut shuffled: Vec<DeltaValue> =
+            [2.0, -f64::NAN, -1.5, f64::INFINITY, -0.0, f64::NEG_INFINITY]
+                .iter()
+                .map(|n| DeltaValue::new(&num(*n)))
+                .collect();
+        shuffled.sort();
+        let want: Vec<DeltaValue> = ascending
+            .iter()
+            .map(|n| DeltaValue::new(&num(*n)))
+            .collect();
+        assert_eq!(shuffled, want);
+        assert_eq!(
+            shuffled[2].value().as_f64().map(f64::to_bits),
+            Some(0),
+            "and the zero among them reads as +0.0"
+        );
+    }
+
+    #[test]
+    fn a_wrapped_value_reads_back_with_the_canonical_nan_and_zero() {
+        let nan = DeltaValue::new(&num(-f64::NAN));
+        assert_eq!(
+            nan.value().as_f64().map(f64::to_bits),
+            Some(f64::NAN.to_bits())
+        );
+        let zero = DeltaValue::new(&num(-0.0));
+        assert_eq!(zero.value().as_f64().map(f64::to_bits), Some(0));
+    }
+
+    #[test]
+    fn a_change_that_undoes_an_earlier_one_cancels_it() {
+        let k = key("East", "Revenue");
+        let delta = cell_delta(&[
+            change(&k, None, Some(num(1.0))),
+            change(&k, Some(num(1.0)), Some(num(2.0))),
+            change(&k, Some(num(2.0)), None),
+        ]);
+        assert!(delta.is_zero());
+    }
+
+    #[test]
+    fn moved_records_names_each_record_once_whatever_the_measure() {
+        let delta = cell_delta(&[
+            change(&key("East", "Revenue"), None, Some(num(1.0))),
+            change(&key("East", "Cost"), None, Some(num(2.0))),
+            change(&key("West", "Cost"), Some(num(3.0)), Some(num(4.0))),
+            change(&key("South", "Cost"), Some(num(5.0)), Some(num(5.0))),
+        ]);
+        let mut want = vec![
+            RecordHash::of(&key("East", "Revenue")),
+            RecordHash::of(&key("West", "Revenue")),
+        ];
+        want.sort_unstable();
+        assert_eq!(moved_records(&delta), want, "South wrote nothing new");
+    }
+}

crates/improvise-incremental/src/lib.rs

@@ -1,10 +1,12 @@
 //! The plug: what the evaluator reads, and the oracle that answers it today.
 //! Reasoning: `context/code/improvise-incremental.md`.
 
+mod delta;
 mod eager;
 mod edit;
 mod harness;
 
+pub use delta::{CellDelta, DeltaValue, cell_delta, moved_records};
 pub use eager::Eager;
 pub use edit::Edit;
 pub use harness::{

crates/improvise-incremental/tests/oracle.rs

@@ -7,7 +7,8 @@ use improvise_core::model::cell::{CellKey, CellValue};
 use improvise_core::model::{Aggregation, Model};
 use improvise_core::view::Axis;
 use improvise_incremental::{
-    Derived, Divergence, Eager, Edit, Maintained, Probe, differential, probes, same_value,
+    CellDelta, DeltaValue, Derived, Divergence, Eager, Edit, Maintained, Probe, cell_delta,
+    differential, probes, same_value,
 };
 use proptest::prelude::*;
 use std::path::Path;
@@ -594,6 +595,50 @@ fn any_value() -> impl Strategy<Value = CellValue> {
     ]
 }
 
+/// The store as a Z-set, every cell at weight one.
+fn snapshot(model: &Model) -> CellDelta {
+    model
+        .data
+        .iter_cells()
+        .map(|(k, v)| ((k, DeltaValue::new(v)), 1))
+        .collect()
+}
+
+/// Values the delta must treat as the float rules do: both zeros, several NaNs.
+fn delta_value() -> impl Strategy<Value = CellValue> {
+    prop_oneof![
+        (-2i32..3).prop_map(|n| CellValue::Number(f64::from(n))),
+        Just(CellValue::Number(-0.0)),
+        Just(CellValue::Number(f64::NAN)),
+        Just(CellValue::Number(-f64::NAN)),
+        Just(CellValue::Number(f64::from_bits(0x7ff0_0000_0000_0001))),
+        prop::sample::select(vec!["n", "x"]).prop_map(|t| CellValue::Text(t.into())),
+    ]
+}
+
+fn cell_edit() -> impl Strategy<Value = Edit> {
+    prop_oneof![
+        (any_key(), delta_value()).prop_map(|(k, v)| Edit::SetCell(k, v)),
+        any_key().prop_map(Edit::ClearCell),
+    ]
+}
+
+proptest! {
+    #[test]
+    fn the_cell_delta_of_an_edit_is_the_store_after_minus_the_store_before(
+        edits in prop::collection::vec(cell_edit(), 0..12)
+    ) {
+        let mut model = hand_model();
+        model.data.record_changes();
+        for edit in &edits {
+            let before = snapshot(&model);
+            edit.apply(&mut model);
+            let delta = cell_delta(&model.data.drain_changes());
+            prop_assert_eq!(delta, &snapshot(&model) - &before, "after {:?}", edit);
+        }
+    }
+}
+
 fn any_edit() -> impl Strategy<Value = Edit> {
     prop_oneof![
         (any_key(), any_value()).prop_map(|(k, v)| Edit::SetCell(k, v)),