From 4abeb7829d9eb59795e4ff99bafa441471c7b688 Mon Sep 17 00:00:00 2001 From: "Claude (dbsp-backend)" Date: Sat, 10 Oct 2026 15:23:09 -0700 Subject: [PATCH] fix(incremental): meter the whole-cache recompute and correct the step 5 notes (improvise-shbc) Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- context/code/improvise-incremental.md | 9 ++++--- .../test-evidence/improvise-incremental.md | 8 +++++- .../improvise-incremental/src/incremental.rs | 1 + crates/improvise-incremental/src/shapes.rs | 4 +++ .../improvise-incremental/tests/formulas.rs | 27 +++++++++++++++++++ crates/improvise-incremental/tests/measure.rs | 1 + 6 files changed, 45 insertions(+), 5 deletions(-) diff --git a/context/code/improvise-incremental.md b/context/code/improvise-incremental.md index e63cd9a9..cc00ff53 100644 --- a/context/code/improvise-incremental.md +++ b/context/code/improvise-incremental.md @@ -221,10 +221,11 @@ taken where clause 4 and clause 7 leave a choice; evidence is in ## Step 5: formulas (improvise-shbc) -`Incremental` no longer calls `recompute_formulas`. It keeps the formula cache (`Model::formula_cell`, -`set_formula_cell`, `evaluate_formula_cell`, the three small additions to `improvise-core`) and decides -which cells of it to evaluate again; the evaluator that computes each value is core's own. The ADR -(clauses 5 and 6) is followed in outline, and the places it is not are the reason for this section. +`Incremental` calls `recompute_formulas` only for the documents item 2 calls wide. Otherwise it keeps +the formula cache itself (`Model::formula_cell`, `set_formula_cell`, `clear_formula_cells` and the +now-public `evaluate_formula_cell`, the small additions to `improvise-core`) and decides which cells of +it to evaluate again; the evaluator that computes each value is core's own. The ADR (clauses 5 and 6) +is followed in outline, and the places it is not are the reason for this section. 1. **A stem is the unit, because eager's cache is per stem.** `Stems` (`formulas.rs`) holds, per target category, how many stored cells give each stem (a cell minus that category and the diff --git a/context/test-evidence/improvise-incremental.md b/context/test-evidence/improvise-incremental.md index b2fe4414..7975f1f4 100644 --- a/context/test-evidence/improvise-incremental.md +++ b/context/test-evidence/improvise-incremental.md @@ -453,7 +453,7 @@ One shared text is that text, two are `mixed`, a number among text is `mixed`, a **How the faults were injected.** Twenty-seven faults went into `src/formulas.rs` and `src/incremental.rs`, one at a time, each run against the whole crate and reverted before the next. -A first pass left eight alive; five were a gap in the tests and were closed by the tests below, and +A first pass left eight alive; six were a gap in the tests and were closed by the tests below, and the remaining two (`born-stems-not-evaluated`, `target-category-not-read`) cannot change an answer. What each fault was and the test that names it: - a cell that gives a stem is not enough to evaluate it / no reader is dirtied: @@ -596,3 +596,9 @@ sixteenth pass to see nothing move. ### formulas::tests::what_a_formula_reads_includes_what_the_formulas_it_names_read `Margin` names `Profit`, which names `Cost`; the reads of `Margin` include `Cost`. + +### the_whole_cache_recompute_of_a_wide_document_is_metered + +A document with formulas in two target categories falls back to `recompute_formulas`; the fallback was +doing the work with the meter at zero (Copilot on PR #351). It now counts `Meter::recomputes`, and a +document with one target category counts none. diff --git a/crates/improvise-incremental/src/incremental.rs b/crates/improvise-incremental/src/incremental.rs index 990eaabd..d43e2b2b 100644 --- a/crates/improvise-incremental/src/incremental.rs +++ b/crates/improvise-incremental/src/incremental.rs @@ -225,6 +225,7 @@ impl Incremental { /// cells, recomputes every formula cell as eager does. fn evaluate(&mut self, stems: impl IntoIterator) { if self.stems.is_wide() { + self.work.recomputes += 1; return self.model.recompute_formulas(&self.none_cats); } for stem in stems { diff --git a/crates/improvise-incremental/src/shapes.rs b/crates/improvise-incremental/src/shapes.rs index 8d8efe65..cbe1f3f0 100644 --- a/crates/improvise-incremental/src/shapes.rs +++ b/crates/improvise-incremental/src/shapes.rs @@ -24,6 +24,9 @@ pub struct Meter { pub formula_cells: u64, /// Times the stems were rebuilt from the store, as a formula edit does. pub rebuilds: u64, + /// Whole-cache recomputes by the fallback for documents whose stems read each other. One is + /// every stem's formulas for every pass, so it is counted as an event, not as cells. + pub recomputes: u64, } impl Meter { @@ -36,6 +39,7 @@ impl Meter { stems: self.stems + o.stems, formula_cells: self.formula_cells + o.formula_cells, rebuilds: self.rebuilds + o.rebuilds, + recomputes: self.recomputes + o.recomputes, } } } diff --git a/crates/improvise-incremental/tests/formulas.rs b/crates/improvise-incremental/tests/formulas.rs index c7706ab9..2bc4883f 100644 --- a/crates/improvise-incremental/tests/formulas.rs +++ b/crates/improvise-incremental/tests/formulas.rs @@ -191,6 +191,33 @@ fn two_target_categories_read_each_others_stems_whatever_the_order_they_are_adde } } +#[test] +fn the_whole_cache_recompute_of_a_wide_document_is_metered() { + let wb = formula_workbook(); + let mut backend = Incremental::new(wb.model.clone(), none_of(&wb)); + let edit = bump("East", "A", "Revenue", 3.0); + backend.apply(&edit); + assert_eq!( + backend.meter().recomputes, + 0, + "one target category: by stem" + ); + backend.apply(&formula_edit(14)); + let before = backend.meter(); + backend.apply(&edit); + backend.apply(&bump("East", "A", "Revenue", 4.0)); + let spent = backend.meter(); + assert_eq!( + spent.recomputes - before.recomputes, + 2, + "Region and _Measure targets: wide" + ); + assert_eq!( + spent.stems, before.stems, + "and no stem is evaluated on its own" + ); +} + #[test] fn a_name_in_another_category_reads_another_stems_cell() { let mut wb = formula_workbook(); diff --git a/crates/improvise-incremental/tests/measure.rs b/crates/improvise-incremental/tests/measure.rs index 0c28c50d..85c0bf9d 100644 --- a/crates/improvise-incremental/tests/measure.rs +++ b/crates/improvise-incremental/tests/measure.rs @@ -67,6 +67,7 @@ fn since(before: Meter, after: Meter) -> Meter { stems: after.stems - before.stems, formula_cells: after.formula_cells - before.formula_cells, rebuilds: after.rebuilds - before.rebuilds, + recomputes: after.recomputes - before.recomputes, } }