diff --git a/context/code/improvise-incremental.md b/context/code/improvise-incremental.md index 0791e1de..ac5d592b 100644 --- a/context/code/improvise-incremental.md +++ b/context/code/improvise-incremental.md @@ -407,13 +407,17 @@ from the source's folds, never pushed down; `Derived` stays in this crate. what a database does inside a read transaction and what `DataStore` does already (its log is drained in the step that made the edit). `&self` because a live source is shared with the model that reads it and changes behind it by interior mutability of its own. -2. **`Incremental::over(model, feed, none_cats)`**: the model is put over the feed (`Model::over`) and - keeps no cells of its own. Its categories, formulas and aggregations are the definition; the +2. **`Incremental::over(model, feed, none_cats)`** takes a `SharedFeed`, the alias core exports (a generic + `Arc` could not take it: `S` is `Sized`). The model is put over the feed (`Model::over`), which + drops the cells it came with, so none is left for `get_cell`, persistence or the harness to read. Its categories, formulas and aggregations are the definition; the source holds the data. Shapes and stems are built from the feed as it stands. 3. **`sync()` drains and maintains; `apply_changes(&[CellChange])` maintains changes the source already reflects**, for a source whose changes reach the backend by another route (a trigger table, a channel). `Maintained::apply` on a fed backend takes definition edits (a formula, an aggregation) - and drains: a cell edit is the source's to make, and `Model::set_cell` refuses it in a debug build. + and drains: a cell edit is the source's to make, and `apply` refuses it with an `assert!` in every + build (a release build lost it silently when only `Model::set_cell`'s `debug_assert` stood in the way). + **A feed has one drainer**: a drain is destructive, so two backends over one feed split its changes + (`context/code/source.md`, `ChangeSource`). 4. **Freshness is the source's.** ADR 003 left "polled, triggered, or stale until refreshed" open: `sync` is the poll, and when it is called is the host's decision. Nothing here schedules it. 5. **What a change costs** is what it costs for the owned store: two group updates and two exact-sum diff --git a/context/code/source.md b/context/code/source.md index 056046c4..f30d8d83 100644 --- a/context/code/source.md +++ b/context/code/source.md @@ -98,7 +98,10 @@ The contract that matters is that a source's cells are those as of the last drai now plus the changes drained later counts no change twice or not at all. Why, and how `Incremental` uses it: `context/code/improvise-incremental.md`, step 7. `DataStore` is not a `ChangeSource`: its log is `&mut` and is drained by the owner in the step that made the edit. A `SharedFeed` is the `Arc` a -backend and a model share. +backend and a model share. **A feed has one drainer**: a drain empties the pending list, so two +backends over one feed split the changes between them (measured in review: the same write gave one +backend a Revenue of 128 and the other 38, and the second then disagreed with its own exact reads). +`a_feed_has_one_drainer` pins the behaviour. Kb-style invariant: `invariant(change_source, one_drainer)`. ## Building a `Summary` from an engine's answer diff --git a/context/test-evidence/improvise-incremental.md b/context/test-evidence/improvise-incremental.md index 61011e72..943bd9a3 100644 --- a/context/test-evidence/improvise-incremental.md +++ b/context/test-evidence/improvise-incremental.md @@ -842,3 +842,28 @@ the model not over the feed fails it with the others. `Fed` against `Eager` over random formula and cell edits on the formula fixture (no views, so the binary stays under two seconds). Rivals: `sync` drains nothing, the model not over the feed, a shape built from an empty store and the stems from the owned store all fail it. + +### a_backend_can_be_built_over_the_exported_feed_alias + +Review of #358: `over(Arc)` could not take the `SharedFeed` the same PR exports. The test builds +a backend from a `SharedFeed`; it did not compile against the generic signature. + +### a_populated_model_put_over_a_feed_keeps_no_cells_of_its_own + +Review of #358: `Fed::new` emptied the store before the call, so no test passed a populated model. This +one does, asserts the owned store is empty afterwards and that every harness probe agrees with +`Eager`. Rival: `Model::over` keeping the cells fails it, and only it. + +### a_cell_edit_on_a_fed_backend_is_refused + +`Maintained::apply` with a `SetCell` panics with the backend's own message in every build. Rival: the +assert removed (the test then sees the model's message, not the backend's, and fails). + +### a_cell_clear_on_a_fed_backend_is_refused + +The same for `ClearCell`. + +### a_feed_has_one_drainer + +Review of #358: two backends over one feed; one write; `a.sync(); b.sync()`. The first sees it, the +second does not. This pins the behaviour as documented rather than as a guarantee: a drain is destructive. diff --git a/crates/improvise-core/src/model/source.rs b/crates/improvise-core/src/model/source.rs index 944ae13f..2cd70a26 100644 --- a/crates/improvise-core/src/model/source.rs +++ b/crates/improvise-core/src/model/source.rs @@ -192,8 +192,8 @@ impl Summary { /// its readers (a database) does so through interior mutability of its own. pub type SharedSource = std::sync::Arc; -/// A source that reports its changes. Its cells are the state after the changes it has drained, -/// so a snapshot read now plus the changes drained later counts no change twice or not at all. +/// A source that reports its changes; its cells are the state as of its last drain. +/// A drain is destructive: a feed has one drainer (`context/code/source.md`). pub trait ChangeSource: CellSource { /// The changes since the last drain, oldest first; reads see them from now on. fn drain_changes(&self) -> Vec; diff --git a/crates/improvise-incremental/src/incremental.rs b/crates/improvise-incremental/src/incremental.rs index 52b4bdff..c83d9746 100644 --- a/crates/improvise-incremental/src/incremental.rs +++ b/crates/improvise-incremental/src/incremental.rs @@ -9,11 +9,10 @@ use crate::{Derived, Edit, Maintained, Reading, cell_delta}; use improvise_core::formula::ast::AggFunc; use improvise_core::model::category::MEASURE_CATEGORY; use improvise_core::model::cell::{CellChange, CellKey, CellValue}; -use improvise_core::model::source::{ChangeSource, SharedFeed}; +use improvise_core::model::source::{SharedFeed, SharedSource}; use improvise_core::model::types::{Covers, settled}; use improvise_core::model::{Aggregation, Model}; use std::collections::BTreeMap; -use std::sync::Arc; /// How a stem's formulas are evaluated: in dependency order where one pass is exact, or by /// eager's passes always. The two must agree; `Plain` is the reference. @@ -47,12 +46,9 @@ impl Incremental { /// A backend over `feed`, which holds the cells and reports their changes; `model`'s own /// store is not read. Reasoning: `context/code/improvise-incremental.md`, step 7. - pub fn over( - model: Model, - feed: Arc, - none_cats: Vec, - ) -> Self { - let model = model.over(feed.clone()); + pub fn over(model: Model, feed: SharedFeed, none_cats: Vec) -> Self { + let source: SharedSource = feed.clone(); + let model = model.over(source); Self::start(model, none_cats, Some(feed)) } @@ -390,6 +386,11 @@ impl Derived for Incremental { impl Maintained for Incremental { fn apply(&mut self, edit: &Edit) { + let refused = self.feed.is_some() && edit.key().is_some(); + assert!( + !refused, + "a fed backend is read-only: cell edits go through the source" + ); edit.apply(&mut self.model); let changes = self.drain(); self.absorb(Some(edit), &changes); diff --git a/crates/improvise-incremental/tests/live_source.rs b/crates/improvise-incremental/tests/live_source.rs index ba6a622a..493c3741 100644 --- a/crates/improvise-incremental/tests/live_source.rs +++ b/crates/improvise-incremental/tests/live_source.rs @@ -5,12 +5,14 @@ mod support; use improvise_core::formula::ast::AggFunc; use improvise_core::model::cell::{CellKey, CellValue}; -use improvise_core::model::source::ChangeSource; +use improvise_core::model::source::{ChangeSource, SharedFeed}; use improvise_core::model::{Aggregation, Model}; use improvise_incremental::{ - Derived, Eager, Edit, Maintained, Meter, differential_in, edit_probes, probes, same_value, + Derived, Eager, Edit, Incremental, Maintained, Meter, differential_in, edit_probes, probes, + same_value, }; -use support::live::Fed; +use std::sync::Arc; +use support::live::{Fed, LiveTable}; use support::{corpus, edits_for, hand_model, key}; #[test] @@ -108,7 +110,7 @@ fn row(r: usize, p: usize, m: usize) -> CellKey { ]) } -/// `n` regions by `n` products by 6 months of one measure, as a model with a profit-style formula. +/// `n` regions by `n` products by 6 months of one measure, with no formula. fn table(n: usize) -> Model { let mut model = Model::new("table"); for (cat, prefix, count) in [("Region", "r", n), ("Product", "p", n), ("Month", "m", 6)] { @@ -211,3 +213,72 @@ fn an_overwrite_by_the_same_value_moves_nothing() { assert_eq!(fed.inc.raw_fold(&revenue), before); assert_eq!(before, Some(CellValue::Number(23.0))); } + +#[test] +fn a_backend_can_be_built_over_the_exported_feed_alias() { + let model = hand_model(); + let rows = model.data.iter_cells().map(|(k, v)| (k, v.clone())); + let feed: SharedFeed = Arc::new(LiveTable::of(rows)); + let mut inc = Incremental::over(model.clone(), feed, vec![]); + let revenue = CellKey::new(vec![("_Measure".into(), "Revenue".into())]); + assert_eq!(inc.raw_fold(&revenue), Some(CellValue::Number(23.0))); + inc.sync(); +} + +#[test] +fn a_populated_model_put_over_a_feed_keeps_no_cells_of_its_own() { + let (model, none) = (hand_model(), vec!["Region".to_string()]); + assert!( + model.data.iter_cells().count() > 0, + "the input is populated" + ); + let rows = model.data.iter_cells().map(|(k, v)| (k, v.clone())); + let live = Arc::new(LiveTable::of(rows)); + let inc = Incremental::over(model.clone(), live, none.clone()); + assert_eq!(inc.model().data.iter_cells().count(), 0, "no stale copy"); + assert_eq!(inc.model().get_cell(&key("East", "A", "Revenue")), None); + let eager = Eager::new(model, none); + for probe in probes(eager.model()) { + let (want, got) = (probe.ask(&eager), probe.ask(&inc)); + assert!( + same_value(&want, &got), + "{probe:?}: eager {want:?} != fed {got:?}" + ); + } +} + +#[test] +#[should_panic(expected = "cell edits go through the source")] +fn a_cell_edit_on_a_fed_backend_is_refused() { + let mut fed = Fed::new(&hand_model(), vec![]); + let edit = Edit::SetCell(key("East", "A", "Revenue"), CellValue::Number(1.0)); + fed.inc.apply(&edit); +} + +#[test] +#[should_panic(expected = "cell edits go through the source")] +fn a_cell_clear_on_a_fed_backend_is_refused() { + let mut fed = Fed::new(&hand_model(), vec![]); + fed.inc.apply(&Edit::ClearCell(key("East", "A", "Revenue"))); +} + +#[test] +fn a_feed_has_one_drainer() { + let model = hand_model(); + let rows = model.data.iter_cells().map(|(k, v)| (k, v.clone())); + let live = Arc::new(LiveTable::of(rows)); + let mut a = Incremental::over(model.clone(), live.clone(), vec![]); + let mut b = Incremental::over(model, live.clone(), vec![]); + let revenue = CellKey::new(vec![("_Measure".into(), "Revenue".into())]); + let (before_a, before_b) = (a.raw_fold(&revenue), b.raw_fold(&revenue)); + assert_eq!(before_a, before_b); + live.write(key("East", "A", "Revenue"), Some(CellValue::Number(100.0))); + a.sync(); + b.sync(); + let (after_a, after_b) = (a.raw_fold(&revenue), b.raw_fold(&revenue)); + assert_ne!(after_a, before_a, "the first drainer saw the write"); + assert_eq!( + after_b, before_b, + "the second saw nothing: a drain is destructive" + ); +}