Merge remote-tracking branch 'gh/main' into fix/fy9o-reopened-editor-reseeded

This commit is contained in:
Edward Langley
2026-09-23 08:38:28 -07:00
13 changed files with 1188 additions and 111 deletions
+50
View File
@@ -3515,3 +3515,53 @@ fixed default, not a clock read. The list is checked in both directions. An
unlisted command that diverges fails the test, and so does a listed one that
does not diverge. A stale entry therefore cannot linger, and a new registration
is covered the day it lands.
## A wire write reaches only a cell the host projects as a Record (improvise-ciw8)
`App::resolve_record` answers "which record does this surrogate name". Its
fallback, `record_in_layout`, accepts any key the current layout names, and that
includes a collapsed group's fold and a key the None axis aggregates over. It
has to stay that permissive: it is also the basis check's question, and a record
nobody has stored yet (the empty cell a client is about to fill) is found only
there. But resolving is not the same as being writable. A hand-rolled frame
`SetCell { record: RecordHash::of({Month=Q2}), .. }` was answered `Accepted` and
stored a cell at `Month=Q2`, while the host's own projection called that
position `CellId::Aggregate`. A write under a formula (`Derived`) was accepted
the same way.
So `model_effects` asks `App::unwritable` before a wire `SetCell` or `ClearCell`
builds its effect, and refuses through `effect::refuse`. That is the #181 path:
the refusal marks the batch, `Host::mutating_frame` undoes the frame, the asker
gets `Rejected(MutationRefused)`, and the reason is on the host's status line.
The question it asks is the projection's own, never a second derivation of it:
- A cell a formula owns is refused. That is `cell_id`'s own `Derived` rule,
`formula_for(&key)`. It is asked directly because a stored record may sit on
no position in the host's current view.
- A record found **in the store** is a record, whatever view anyone is on.
`RecordHash` is document-scoped (gotcha 44).
- A record found **only through the layout** is asked of `project::cell_id` at
the position of **its cell**: the one whose key is the whole key the write
names, record and measure together. `Aggregate`, a fold or a sum over a
None-axis category, is refused. It must be the whole key, not the first
position carrying the record (improvise-sjdb). `RecordHash` drops `_Measure`,
so every column of a row carries the same record, and with a collapsed
`_Measure` group the first of them can be the fold. That refused the empty
`Units` cell beside a `Money` fold, which the host itself projects as a
`Record`. A write whose whole key the host's grid does not draw is not
refused here, as it was not before this rule existed.
**Why the `vec![]` arm stays.** A reviewer asked whether a `SetCell` whose cell
does not resolve at all, which `model_effects` turns into `vec![]` and so
applies nothing, should also be a `Rejection`. Through a `Host`, it already is,
before that arm is reached. A lone write naming a record that never existed gets
`RecordGone` up front, whether or not the basis claims it, because the create
exemption covers only a *later* command in a frame
(`a_wire_write_to_a_record_that_never_existed_is_rejected`). A later write whose
record nothing in the frame made resolvable gets `ChainBroken` from
`broken_chain`
(`a_frame_whose_write_would_land_nowhere_is_refused_rather_than_dropped`). So
the arm is unreachable from the wire. Making it a refusal would add a second
answer to "does this resolve" beside `broken_chain`, and that answer could only
ever fire after the first had already refused. It is left as the mutation
path's own behaviour for callers that do not go through the session door.
+69 -13
View File
@@ -869,11 +869,14 @@ an involution without anything being taught about involution, and neither
### The cell
`Model::evaluate_aggregated` expands, at its head, any coordinate that names a
group rather than an item into one key per member (a cross product where several
coordinates do). Doing it in the **model** is what makes `display_text`, prune,
column totals, `grid_of`, search, yank and CSV export all follow with no change.
Three decisions:
`Model::evaluate_covering` expands, at its head, any coordinate that names a group
rather than an item into one key per covered member (a cross product where
several coordinates do). The grid reads every pivot key through
`GridLayout::evaluate`, which passes the members it draws. Since
improvise-eccl/f2oo, display text, pruning, column totals, `grid_of`, search,
yank and CSV export all go through that one evaluation, rather than each calling
`Model::evaluate_aggregated`. That remains the view-less answer, and it covers
every member. Five decisions:
- **Membership is read off each item's own `group` field, never off
`Category::groups`.** `add_item_in_group` registers no `Group`; that list is
@@ -882,14 +885,67 @@ Three decisions:
- **An item with the same name as the group wins.** `items.contains_key`
short-circuits the expansion, so an existing document with an item `Q1` reads
exactly as before.
- **Stored values fold once over every record the group covers; a derived value
folds its members' results.** Folding the members' *aggregates* is right for
`Sum` and wrong for `Avg`/`Count` under `Axis::None` — `avg(avg(2,4), 9) = 6`
where the group's average is `5`. A formula target has no stored records to
pool, so its members' evaluated results are folded with the measure's
aggregation; for a ratio formula that is the usual pivot-table limitation, not
a new one. Hidden items are included in the fold, as on `Axis::None`: hiding
decides what is drawn, not what an aggregate covers.
- **Stored values fold once over every record the group covers.** Folding the
members' *aggregates* is right for `Sum` and wrong for `Avg`/`Count` under
`Axis::None`: `avg(avg(2,4), 9) = 6` where the group's average is `5`.
- **A formula is evaluated over the fold, never folded itself** (improvise-eccl).
This bullet used to say a formula target "has no stored records to pool", so
its members' results were folded with the measure's aggregation, and it called
that "the usual pivot-table limitation". **The premise was wrong.** A formula's
*inputs* are stored and can be pooled, and the model already evaluated a
formula over pooled inputs whenever a category sat on `Axis::None`:
`recompute_formulas` strips the None categories from the stems. Folding the
members' results summed ratios instead, so `Margin = (Revenue - Cost) / Revenue`
at 0.5 and 0.9 showed **1.4** on the Q1 row where Month-on-None showed **0.8**.
Spreadsheet calculated fields do not have that limitation either. The formula
is now evaluated at the group key, with each reference read through the fold
(`Reach::Fold` in `model/types.rs`):
- a raw measure reads the stored fold;
- another formula target is evaluated over the fold in turn;
- an aggregate inside the formula (`SUM`, `MAX`, …) pools every covered
member's records. `MAX(Revenue)` had been *summing* the members' maxima.
The collapsed row now agrees with the None-axis answer. **Test with a ratio**:
a sum agrees under both rules, which is how this was missed.
- **The folded category reads as absent** (improvise-lnkb, the orchestrator's
ruling). `Month IS Q1` is false on the Q1 fold, and a formula's own
`WHERE Month = "Q1"` does not apply there. Both answers are what the None axis
gives, and what an inline `SUM(… WHERE Month = "Q1")` already gave. This
PR's first version said `IS` saw the group's name and called that "stated
rather than hidden". That made the fold row the one place a formula could
observe collapse state, breaking this section's own "fold = None over the
drawn members" rule. `Model::read_coordinate` is the one reading of a
coordinate by name at a fold.
- A formula with its own `WHERE Month = "Jan"` shows **empty** on the Q1 fold,
because the fold is not Jan. The None axis gives the same.
- **A fold evaluates each formula once** (improvise-o1y0). A reference to
another formula target is evaluated over the fold in turn, so without memory
a doubling chain re-evaluates exponentially. The first version did exactly
that: a chain of eight took 511 formula evaluations for one cell, and the
reviewer measured 9.83 s on an 18k-cell grid. `evaluate_covering` now carries
a memo for one evaluation, and a key met while still in progress is reported
`circular`. Measured on an Apple M4 Max in release, on 100 products × 12
months folded to quarters with the F0…F8 chain (4,000 cells): 0.94–1.04 s
without the memo, about 60 ms with it, and about 10 ms on gh/main. gh/main
was cheap because it read the members out of the formula cache, which is
what gave the wrong ratios. The remaining ~6× is each column's cell
re-deriving the chain below it (45 evaluations per row where 9 would do). A
memo scoped to the whole grid would remove it; it is not built.
- **A fold covers the members the grid draws** (improvise-f2oo). Hidden members
are not in it. So a fold is exactly the rows it replaces, the total of a
**stored** measure under Sum or Count does not move on `z`, and a group whose
every member is hidden has no row. Under Avg, Min or Max a stored measure's
total does move, and so does a **formula** column's (PR #203's review measured
Margin 1.90 expanded, 1.30 collapsed). The strip sums the rows' values rather
than folding the column the way a fold does, so it sums a Max row's maximum
and a formula row's result. That is improvise-wm20, and it is not this rule. That last
point was already true, and it contradicted the old rule, under which a hidden
value appeared only while a sibling was visible. This deliberately **differs
from `Axis::None`**, where hidden items still count, because the two replace
different things. A fold replaces *rows the user sees*. A None axis replaces a
*dimension*, and hiding an item on it hides nothing that was drawn. The view's
hidden set reaches the model as the `covers` predicate `GridLayout::evaluate`
passes, since the model knows no view.
### Settability and drill
+74
View File
@@ -5717,6 +5717,17 @@ cannot see the feature removed from `improvise-command` alone, because
`improvise-io`, a normal dependency, enables it in the same build. That case is
held by `every_crate_that_can_parse_json_enables_float_roundtrip` (`tooling.md`).
### the_grid_and_a_csv_export_agree_on_a_fold_row
The projection-level pin PR #203's review asked to keep. The review's own
scratch test is made permanent here. Q1 = {Jan, Feb, Mar} with March hidden and
funded (50/50), so a fold that covered hidden members would read 90 / 58 / 0.36.
`grid_of` (what a client is sent) and `export_csv_to` (what a file gets) must
both read `40.00, 8.00, 0.80` on the Q1 row. The two go through different
producers (`viewport::grid_body` and `render_csv`), and both reach
`GridLayout::evaluate`. Rival: that evaluation covering every member. This test
fails with `["90.00", "58.00", "0.36"]`.
### p_onto_a_sparse_record_writes_it
improvise-q9eg, a regression introduced by PR #197 and found in its review. In
@@ -5787,6 +5798,69 @@ on `Food/Jan/Profit`, where there is nothing to clear, and
`Clothing/Jan/Profit = 5` survives. Injected alone into a full workspace run,
the rival fails this test and nothing else.
### a_wire_write_to_a_collapsed_fold_is_rejected_and_stores_nothing
improvise-ciw8, through a real `Host` frame. Q2 is collapsed, and a hand-rolled
client sends `SetCell` (then, separately, `ClearCell`) for `{Month=Q2, _Measure=Cost}`
with a caught-up basis. At `ea119bd9` both were answered `Accepted`, and the
`SetCell` stored `Month=Q2, _Measure=Cost = 7`, the non-item coordinate
improvise-gw18 exists to prevent. The test asserts `Rejected(MutationRefused)`
and an unchanged document for both. Rival: the aggregate refusal disabled. This
test and the None-axis test fail. The control is
`a_wire_write_to_a_stored_cell_beside_a_fold_is_accepted`, so a host that refused
every wire write cannot pass.
### a_wire_write_under_a_formula_is_rejected_and_stores_nothing
improvise-ciw8's `Derived` arm: `SetCell` on Feb × Profit, where
`Profit = Revenue - Cost`. At `ea119bd9` it was accepted and stored a value under
the formula. Rival: the formula refusal disabled. Only this test fails.
### a_wire_write_to_a_none_axis_aggregate_is_rejected_and_stores_nothing
improvise-ciw8's other `Aggregate`. With `Product` on `Axis::None`, the grid's
key `{Region=South, _Measure=Revenue}` is a partial key that no stored record
has, so it resolves only through the layout, where `cell_id` says `Aggregate`.
At `ea119bd9` it was accepted and stored the partial key. It fails under the
same rival as the fold test.
### a_wire_write_to_a_record_that_never_existed_is_rejected
The evidence for keeping `model_effects`' `vec![]` arm rather than turning it
into a refusal (`context/code/command.md`, "A wire write reaches only a cell the
host projects as a Record"). A lone write naming a record that never existed is
answered `RecordGone` before anything runs, both when a caught-up basis claims
it and when a hand-rolled basis omits it. The second half was expected to be
`ChainBroken`, and the test said otherwise: the create exemption covers only
commands after the first in a frame. That surprise is why it is asserted rather
than argued.
### an_empty_cell_beside_a_collapsed_measure_group_is_writable
improvise-sjdb, a regression PR #217 introduced and its review found. `_Measure`
is on Column with Revenue and Cost in a collapsed group `Money` and `Units`
ungrouped, and the Feb row is empty. The host projects Feb × Units as a
`Record`. A caught-up `SetCell` on it was answered `Rejected(MutationRefused)`
at `bc060f67`. It was accepted before #217. `aggregate_refusal` found the
unstored record by `RecordHash` alone, which drops `_Measure`, so it judged the
record at the first column on Feb's row, the `Money` fold, which `cell_id` calls
`Aggregate`. The setup asserts that the fold comes before `Units` on the row and
that the target projects as `Record`, so a fixture where the first column is
harmless cannot pass. The fix looks the cell up by its whole key. The rival is
the old lookup by record, which is this test's red commit, `6e021699`.
### a_stored_record_the_hosts_view_aggregates_is_writable
improvise-sjdb, from the #217 review. South/Revenue is stored, then `Product` is
added on `Axis::None` in the host's view, so the host projects South's position
as `Aggregate`. A stored record is a record wherever it is drawn, because
`RecordHash` is document-scoped (gotcha 44). A records-mode client, or one on
another view, may write it. The setup asserts the host really does project
`Aggregate` there, so the test cannot pass on a view that draws South plainly.
Rival: stored records sent through `cell_id` like unstored ones. That rival
fails this test and nothing else in `improvise-command`. Before this test,
nothing in the suite saw it.
### a_fresh_sessions_slice_is_the_hosts_startup_slice
improvise-v8ab. A fresh session's slice is built twice: `App::with_storage`
+107 -2
View File
@@ -1346,8 +1346,8 @@ this fixture and the value is absent — this test, the next, and
Under `Axis::None` (`Store`), `Avg` and `Count` over `Jan/A=2, Jan/B=4, Feb/A=9`.
The group's average is `5` and its count `3`; folding the members' aggregates gives
`avg(3, 9) = 6` and `count(2, 1) = 2`. Rival: always fold members' results (the
formula-target branch taken for stored values) — this test fails. `Sum` cannot see
`avg(3, 9) = 6` and `count(2, 1) = 2`. Rival: always fold members' results (what the
formula-target branch did before improvise-eccl) — this test fails. `Sum` cannot see
the difference, which is why the fixture sets `measure_agg`.
### model::types::model_tests::an_item_named_like_a_group_is_read_as_the_item
@@ -1412,3 +1412,108 @@ are exercised. It also lists the key's coordinates out of sorted order, which ch
that the steps follow `CellKey`'s canonical order. Rival: `Axis::Page` in place of
`Axis::Filter` for a fixed coordinate, which reopens improvise-dz8 (a drilled
coordinate the user could page away from). This test fails.
### a_folds_ratio_formula_is_the_ratio_of_its_folded_inputs
improvise-eccl. `Margin = (Revenue - Cost) / Revenue` with Jan `10/5` and Feb `30/3`
under `Q1`, and Apr in `Q2` with no data, so the Q1 fold is row 1. The member ratios
are 0.5 and 0.9, so the three candidate rules give three different answers: summed
1.4, averaged 0.7, pooled `(40 - 8) / 40` 0.8. The expected value is the **pooled**
one, and it is also asserted equal to the same document with Month on `Axis::None`,
which is the ticket's criterion. The None-axis document works because Apr carries
no data, so the None axis pools exactly Q1's months. At `a164726f` the fold read
**1.4**. The ticket's own fixture (0.5 and 0.5) would have separated sum from
pool, but not mean from pool. The second formula set chains
(`Profit = Revenue - Cost`, `Margin = Profit / Revenue`), so a reference to
another formula target is read over the fold as well. A rule that folds a formula
ref's members while pooling raw refs fails there.
### a_folds_aggregate_formula_pools_its_members_records
The guard on the other side of improvise-eccl. Once a formula at a fold is
evaluated at the group key, an aggregate inside it (`SUM`, `MAX`) must pool every
covered member's records, because `Month=Q1` matches no record. `MAX(Revenue)` is
the discriminating case. It **failed before the change too**: the old rule folded
the members' `MAX`es with the measure's `Sum`, giving 40 where the answer is 30.
`SUM` alone agrees under the old rule and the new one. Rival: `contexts_of` stops
pooling at a fold (`Reach::Fold { .. } => Vec::new()`). This test alone fails.
### collapsing_a_group_with_a_hidden_member_leaves_the_total_where_it_was
improvise-f2oo. `Q1 = {Jan 3, Feb 4}`, `Q2 = {Apr 100}`, Feb hidden. Expanded, the
drawn rows total 103. At `a164726f` collapsing gave a Q1 fold of 7 and a total of
**107**: `z` moved the total. The ruling: a fold covers the members the grid
draws. The test asserts 103 both ways and a fold of 3. It then hides Jan too and
asserts that there is no Q1 row and a total of 100. That half already held, and it
is the inconsistency the ticket named: before, a hidden value appeared only while
a sibling was visible. Rival: `GridLayout::evaluate` covering every member
(`&|_, _| true`). This test alone fails, with 107.
### a_folds_aggregate_filtered_on_the_folded_category_counts_each_record_once
Found by the ChatGPT Codex review of PR #203, at head `14acf46d`. Pooling an
aggregate over a fold's member contexts ran `agg_values` once per member, and an
inline `WHERE` on the **folded** category overwrites that coordinate, as it does
at a record. Every member therefore became the same record set.
`SUM(Revenue WHERE Month = "Jan")` on the Q1 fold read **20**, Jan twice. The
test asserts 10, and equality with the Month-on-None document. The fix dedupes
the member contexts after the filter is applied, so each record set is pooled
once. The rival is the pre-fix code, which read 20. `a_folds_aggregate_formula_pools_its_members_records`
could not see this, because none of its aggregates carried a filter.
### a_fold_evaluates_each_formula_it_reaches_once
improvise-o1y0, from PR #203's review. A reference from a fold's formula to
another formula target is evaluated over the fold in turn, with nothing
remembered, so a doubling chain `F{i} = F{i-1} + F{i-1}` re-evaluated
exponentially. The test **counts** formula evaluations rather than timing them,
through a test-only thread-local counter. At `2a4f35d7` evaluating `F8` on one
fold took **511** evaluations (2⁹ − 1); with the memo it takes 9. It also
asserts the chain's value (7 × 256), so a memo that returned the wrong entry
cannot pass. Rival: keep the in-progress marker but drop finished values from
the memo (`.filter(Option::is_none)`). This test fails with 511.
### a_cycle_at_a_fold_is_still_circular
improvise-o1y0's other half. `P = Q + 1`, `Q = P + 1` at a fold must read
`Error("circular")`. The memo's in-progress marker is what answers it now,
because the depth bound the fold path used before is gone. Without the marker
the evaluation recurses until the stack runs out, which is why the marker is
there rather than being an optimisation.
### one_formula_at_two_fold_keys_is_evaluated_at_each
improvise-d7aj, found by the adversarial re-review of PR #203 at `b3379d1a`. The
fold memo in `fold_formula_once` is keyed by the whole `CellKey`, and nothing
pinned that: both tests above keep one key context throughout, so a memo keyed
by the formula alone passed the whole workspace. A reference to an item of
**another** category moves the key while `_Measure` names the same formula.
Stores A (Jan 3, Feb 4) and B (Jan 100, Feb 200), `G = Revenue * 2`,
`F = IF(Revenue > 50, G, G + B)`. At `(Q1, A, F)` Revenue is 7, so F is
`G + B`: G at A is 14, and `B` reads F at `(Q1, B)`, where Revenue is 300, so
that F is G at B, 600. The answer is **614**. Rival: the memo keyed by
`formula.target` alone. F is still in progress at `(Q1, A)` when F at `(Q1, B)`
is asked, so that key reads `circular`. Injected into a full workspace run, it
fails this test and nothing else. The reviewer's probe, now committed.
### a_diamond_at_a_fold_is_not_circular
improvise-d7aj's second case, which passed at `b3379d1a` and was pinned by
nothing. `A1 = B2 + C3`, `B2 = D * 2`, `C3 = D * 3`, `D = Revenue` reaches D by
two paths in one fold evaluation, so the second reach must find D *finished*
and not *in progress*. It asserts **35** over D = 7 and exactly four
evaluations, so a memo that re-evaluates D passes neither. Rival: never store a
finished value, so only the in-progress marker stays. This test fails with
`circular`, and so does `a_fold_evaluates_each_formula_it_reaches_once`, whose
doubling chain is also a diamond at every step. So this test is a named pin of
that shape rather than the only guard against it.
### a_fold_answers_is_and_where_on_its_category_as_the_none_axis_does
improvise-lnkb, ruled by the orchestrator: on a fold, the folded category reads
as absent. `Flag = Month IS "Q1"` and `Marked = Revenue * 2 WHERE Month = "Q1"`
are each compared with the same document with Month on `Axis::None`. At
`689c7665` `Flag` read **1** on the fold and 0 on None. The final assertion pins
`Flag` at `Some(0.0)`, false rather than absent, so a fix that dropped the cell
cannot pass. Rival: `read_coordinate` ignoring the reach (`if false && …`).
This test fails.
@@ -1,4 +1,3 @@
use crate::model::Aggregation;
use crate::model::cell::{CellKey, CellValue};
use crate::ui::effect::{self, Effect, ViewTarget};
use crate::view::synthetic_record_info;
@@ -585,9 +584,7 @@ impl Cmd for YankCell {
// A records cell is the layout's one resolution, as the browser yanks it (gotcha 1).
let value = match synthetic_record_info(&self.key) {
Some((row, col)) => ctx.layout.records_value(row, col),
None => ctx
.model
.evaluate_aggregated(&self.key, Aggregation::of(ctx.none_cats())),
None => ctx.layout.evaluate(ctx.model, &self.key),
};
vec![
Box::new(effect::SetYanked(value)),
@@ -1,4 +1,3 @@
use crate::model::Aggregation;
use crate::model::cell::CellValue;
use crate::ui::app::AppMode;
use crate::ui::effect::{self, Effect, Panel};
@@ -113,10 +112,7 @@ impl Cmd for SearchNavigate {
Some(k) => k,
None => return false,
};
let s = match ctx
.model
.evaluate_aggregated(&key, Aggregation::of(ctx.none_cats()))
{
let s = match ctx.layout.evaluate(ctx.model, &key) {
Some(CellValue::Number(n)) => format!("{n}"),
Some(CellValue::Text(t)) => t,
Some(CellValue::Error(e)) => format!("ERR:{e}"),
+43 -3
View File
@@ -27,7 +27,7 @@ use crate::ui::viewport::{compute_col_widths, compute_row_header_width, compute_
use crate::view::{GridLayout, View};
use crate::workbook::Workbook;
use improvise_core::model::record::{AggregateHash, RecordHash};
use improvise_protocol::RecordCell;
use improvise_protocol::{CellId, RecordCell};
/// A cell's coordinate tuple with `_Measure` dropped: the unit of atomicity
/// ADR 001 settles on, and not the cell — see `context/code/command.md`.
@@ -576,16 +576,56 @@ impl App {
.map(|key| record_of(&key))
}
/// Why a wire write to `cell` is refused: the host itself projects it as no
/// `CellId::Record` (improvise-ciw8, ADR 002). `None` for a writable cell.
pub fn unwritable(&self, cell: &RecordCell) -> Option<String> {
let key = self.resolve_cell(cell)?;
let model = &self.model_state.workbook.model;
let derived = model.formula_for(&key).map(|f| {
format!(
"{} is computed by a formula — edit the formula, not the cell",
f.target
)
});
derived.or_else(|| self.aggregate_refusal(cell.record, &key))
}
/// A record the store does not hold is known only by its layout position, and
/// is a record only where the projection at ITS CELL says so (improvise-sjdb).
fn aggregate_refusal(&self, hash: RecordHash, key: &CellKey) -> Option<String> {
let stored = self.model_state.workbook.model.data.resolve_record(hash);
let (row, col) = stored.map_or_else(|| self.cell_position(key), |_| None)?;
let projected =
crate::ui::project::cell_id(&self.model_state.workbook.model, &self.layout, row, col);
matches!(projected, Some(CellId::Aggregate(_))).then(|| {
"an aggregate combines several records, so there is no one cell to write".into()
})
}
/// Where in the current layout the cell `key` sits, if anywhere. By the whole
/// key: a `RecordHash` drops `_Measure`, so a row's columns all share one.
fn cell_position(&self, key: &CellKey) -> Option<(usize, usize)> {
self.layout_cells()
.find(|(_, _, at)| at == key)
.map(|(row, col, _)| (row, col))
}
/// Every cell key the current grid names. Records mode is excluded: its
/// `cell_key` carries *positions*, the trap ADR 002 rejected `_Index` for.
fn layout_cell_keys(&self) -> impl Iterator<Item = CellKey> + '_ {
self.layout_cells().map(|(_, _, key)| key)
}
/// [`Self::layout_cell_keys`] with each key's position.
fn layout_cells(&self) -> impl Iterator<Item = (usize, usize, CellKey)> + '_ {
let layout = &self.layout;
let rows = match layout.is_records_mode() {
true => 0,
false => layout.row_count(),
};
(0..rows)
.flat_map(move |r| (0..layout.col_count()).filter_map(move |c| layout.cell_key(r, c)))
(0..rows).flat_map(move |r| {
(0..layout.col_count()).filter_map(move |c| layout.cell_key(r, c).map(|k| (r, c, k)))
})
}
/// Point **this client's** slice at `name`, if the document has such a view,
+254
View File
@@ -1358,6 +1358,260 @@ mod tests {
);
}
fn month_cell(month: &str, measure: &str) -> CellKey {
CellKey::new(vec![
("Month".to_string(), month.to_string()),
("_Measure".to_string(), measure.to_string()),
])
}
/// Months in two quarters on Row, Revenue/Cost and `Profit = Revenue - Cost` on
/// Column, every month funded, Q2 collapsed: rows `[Jan, Feb, Q2]`.
fn host_on_a_fold() -> Host {
let mut wb = Workbook::new("T");
wb.add_category("Month").unwrap();
for (month, quarter) in [("Jan", "Q1"), ("Feb", "Q1"), ("Apr", "Q2"), ("May", "Q2")] {
wb.add_item_in_group("Month", month, quarter).unwrap();
for measure in ["Revenue", "Cost"] {
wb.add_item("_Measure", measure).unwrap();
let value = CellValue::Number(10.0);
wb.model.set_cell(month_cell(month, measure), value);
}
}
let profit = crate::formula::parse_formula("Profit = Revenue - Cost", "_Measure");
wb.model.add_formula(profit.unwrap());
wb.active_view_mut().set_axis("Month", Axis::Row);
wb.active_view_mut().set_axis("_Measure", Axis::Column);
wb.active_view_mut().toggle_group_collapse("Month", "Q2");
let mut app = App::new(wb, None);
app.rebuild_layout();
assert!(
app.layout.folds_a_group(2, 1),
"setup: row 2 is the Q2 fold"
);
Host::new(app)
}
/// One wire write, as a hand-rolled client composes it: a surrogate built from
/// a key, and a basis claiming whatever the host says is current.
fn wire_write(host: &mut Host, key: &CellKey, clear: bool) -> Vec<Addressed> {
use improvise_protocol::ModelCommand;
let cell = RecordCell {
record: RecordHash::of(key),
measure: key.get("_Measure").map(String::from),
};
let cmd = match clear {
true => ModelCommand::ClearCell(cell),
false => ModelCommand::SetCell {
cell,
value: CellValue::Number(7.0),
},
};
let (asker, _) = host.connect(10, 10);
let frame = crate::ui::reduce::caught_up(host.app(), cmd);
host.handle(asker, ClientFrame::Commands(vec![frame]))
}
fn refused(answer: &[Addressed]) -> bool {
answer
.iter()
.any(|a| matches!(a.msg, ServerFrame::Rejected(Rejection::MutationRefused)))
}
fn document_text(host: &Host) -> String {
crate::persistence::format_md(&host.app().model_state.workbook)
}
/// improvise-ciw8 — see context/test-evidence/command.md.
#[test]
fn a_wire_write_to_a_collapsed_fold_is_rejected_and_stores_nothing() {
for clear in [false, true] {
let mut host = host_on_a_fold();
let before = document_text(&host);
let answer = wire_write(&mut host, &month_cell("Q2", "Cost"), clear);
assert!(refused(&answer), "clear={clear}: not refused: {answer:?}");
assert_eq!(
document_text(&host),
before,
"clear={clear}: the fold was written"
);
}
}
/// improvise-ciw8: a formula's result is `CellId::Derived`, no write's target.
#[test]
fn a_wire_write_under_a_formula_is_rejected_and_stores_nothing() {
let mut host = host_on_a_fold();
let before = document_text(&host);
let answer = wire_write(&mut host, &month_cell("Feb", "Profit"), false);
assert!(refused(&answer), "not refused: {answer:?}");
assert_eq!(
document_text(&host),
before,
"a value was stored under the formula"
);
}
/// improvise-ciw8: a key the None axis aggregates over is `CellId::Aggregate`.
#[test]
fn a_wire_write_to_a_none_axis_aggregate_is_rejected_and_stores_nothing() {
let mut host = host();
host.app
.model_state
.workbook
.add_category("Product")
.unwrap();
host.app
.model_state
.workbook
.add_item("Product", "Widget")
.unwrap();
let view = host.app.model_state.workbook.active_view_mut();
view.set_axis("Product", Axis::None);
host.app.rebuild_layout();
let before = document_text(&host);
let answer = wire_write(&mut host, &cell("South"), false);
assert!(refused(&answer), "not refused: {answer:?}");
assert_eq!(document_text(&host), before, "the partial key was written");
}
/// improvise-ciw8's second question: a write naming a record that never existed
/// is refused by the frame, not dropped — see context/test-evidence/command.md.
#[test]
fn a_wire_write_to_a_record_that_never_existed_is_rejected() {
use improvise_protocol::ModelCommand;
let mut host = host_on_a_fold();
let before = document_text(&host);
// Claimed by a caught-up basis: structural staleness, answered up front.
let answer = wire_write(&mut host, &month_cell("Nowhere", "Cost"), false);
assert!(
answer
.iter()
.any(|a| matches!(a.msg, ServerFrame::Rejected(Rejection::RecordGone(_)))),
"claimed: {answer:?}"
);
// Unclaimed: the create exemption covers only LATER commands in a frame,
// so a lone write is answered up front too.
let key = month_cell("Nowhere", "Cost");
let cmd = ModelCommand::SetCell {
cell: RecordCell {
record: RecordHash::of(&key),
measure: Some("Cost".into()),
},
value: CellValue::Number(7.0),
};
let basis = improvise_protocol::Basis {
document_generation: host.app().document_generation(),
layout_version: host.app().layout_version(),
records: vec![],
};
let (asker, _) = host.connect(10, 10);
let frame = ClientFrame::Commands(vec![Command::Model { basis, cmd }]);
let answer = host.handle(asker, frame);
assert!(
answer
.iter()
.any(|a| matches!(a.msg, ServerFrame::Rejected(Rejection::RecordGone(_)))),
"unclaimed: {answer:?}"
);
assert_eq!(document_text(&host), before);
}
/// The control for the three above: the stored cell beside the fold is written.
#[test]
fn a_wire_write_to_a_stored_cell_beside_a_fold_is_accepted() {
let mut host = host_on_a_fold();
let answer = wire_write(&mut host, &month_cell("Feb", "Cost"), false);
assert!(
answer
.iter()
.any(|a| matches!(a.msg, ServerFrame::Accepted)),
"{answer:?}"
);
let model = &host.app().model_state.workbook.model;
let value = model.get_cell(&month_cell("Feb", "Cost"));
assert_eq!(value, Some(&CellValue::Number(7.0)));
}
/// improvise-sjdb: an empty record is judged at its own cell, not at the first
/// column carrying it — see context/test-evidence/command.md.
#[test]
fn an_empty_cell_beside_a_collapsed_measure_group_is_writable() {
let mut wb = Workbook::new("T");
wb.add_category("Month").unwrap();
for month in ["Jan", "Feb"] {
wb.add_item("Month", month).unwrap();
}
for measure in ["Revenue", "Cost"] {
wb.add_item_in_group("_Measure", measure, "Money").unwrap();
}
wb.add_item("_Measure", "Units").unwrap();
let jan = month_cell("Jan", "Revenue");
wb.model.set_cell(jan, CellValue::Number(10.0));
wb.active_view_mut().set_axis("Month", Axis::Row);
wb.active_view_mut().set_axis("_Measure", Axis::Column);
wb.active_view_mut()
.toggle_group_collapse("_Measure", "Money");
let mut app = App::new(wb, None);
app.rebuild_layout();
let target = month_cell("Feb", "Units");
let units = (0..app.layout.col_count())
.find(|&c| app.layout.cell_key(1, c).as_ref() == Some(&target))
.expect("setup: Feb x Units is drawn");
let fold = (0..app.layout.col_count())
.find(|&c| app.layout.folds_a_group(1, c))
.expect("setup: a folded measure column");
assert!(fold < units, "setup: the fold comes first on Feb's row");
let id =
crate::ui::project::cell_id(&app.model_state.workbook.model, &app.layout, 1, units);
let projected = matches!(id, Some(improvise_protocol::CellId::Record(_)));
assert!(projected, "setup: Feb x Units projects as {id:?}");
let mut host = Host::new(app);
let answer = wire_write(&mut host, &target, false);
let accepted = answer
.iter()
.any(|a| matches!(a.msg, ServerFrame::Accepted));
assert!(accepted, "a projected Record was refused: {answer:?}");
let model = &host.app().model_state.workbook.model;
assert_eq!(model.get_cell(&target), Some(&CellValue::Number(7.0)));
}
/// A STORED record is writable whatever the host's view does to it — see
/// context/test-evidence/command.md (improvise-sjdb).
#[test]
fn a_stored_record_the_hosts_view_aggregates_is_writable() {
let mut wb = workbook();
wb.model.set_cell(cell("South"), CellValue::Number(1.0));
wb.add_category("Product").unwrap();
wb.add_item("Product", "Widget").unwrap();
wb.active_view_mut().set_axis("Product", Axis::None);
let mut app = App::new(wb, None);
app.rebuild_layout();
let south = (0..app.layout.row_count())
.flat_map(|r| (0..app.layout.col_count()).map(move |c| (r, c)))
.find(|&(r, c)| app.layout.cell_key(r, c).as_ref() == Some(&cell("South")))
.expect("setup: South's cell is drawn");
let id = crate::ui::project::cell_id(
&app.model_state.workbook.model,
&app.layout,
south.0,
south.1,
);
let aggregate = matches!(id, Some(improvise_protocol::CellId::Aggregate(_)));
assert!(aggregate, "setup: the host's view aggregates South: {id:?}");
let mut host = Host::new(app);
let answer = wire_write(&mut host, &cell("South"), false);
let accepted = answer
.iter()
.any(|a| matches!(a.msg, ServerFrame::Accepted));
assert!(accepted, "a stored record was refused: {answer:?}");
let model = &host.app().model_state.workbook.model;
assert_eq!(
model.get_cell(&cell("South")),
Some(&CellValue::Number(7.0))
);
}
/// Clause 4 must not fire on a frame that merely names a record it is
/// creating. See `context/test-evidence/command.md`.
#[test]
@@ -5293,6 +5293,66 @@ mod cell_identity {
"another record's basis was invalidated for nothing"
);
}
/// improvise-eccl/f2oo at the projection boundary: the grid a client is sent and
/// the CSV an export writes agree on a fold row — see the evidence file.
#[test]
fn the_grid_and_a_csv_export_agree_on_a_fold_row() {
let mut wb = Workbook::new("T");
wb.add_category("Month").unwrap();
wb.add_item_in_group("Month", "Apr", "Q2").unwrap();
for month in ["Jan", "Feb", "Mar"] {
wb.add_item_in_group("Month", month, "Q1").unwrap();
}
for (month, revenue, cost) in [("Jan", 10.0, 5.0), ("Feb", 30.0, 3.0), ("Mar", 50.0, 50.0)]
{
for (measure, n) in [("Revenue", revenue), ("Cost", cost)] {
wb.add_item("_Measure", measure).unwrap();
let key = CellKey::new(vec![
("Month".into(), month.into()),
("_Measure".into(), measure.into()),
]);
wb.model.set_cell(key, CellValue::Number(n));
}
}
wb.model.add_formula(
crate::formula::parse_formula("Margin = (Revenue - Cost) / Revenue", "_Measure")
.unwrap(),
);
wb.active_view_mut().set_axis("Month", Axis::Row);
wb.active_view_mut().set_axis("_Measure", Axis::Column);
wb.active_view_mut().hide_item("Month", "Mar");
wb.active_view_mut().toggle_group_collapse("Month", "Q1");
let app = App::new(wb, None);
let grid = grid_of(&app, WHOLE);
let q1 = grid.row_frame.leaf_labels().iter().position(|l| l == "Q1");
let q1 = q1.expect("a Q1 fold row");
let drawn: Vec<String> = (0..grid.col_frame.leaf_labels().len())
.map(|c| grid.cell(q1, c).map(|d| d.text.clone()).unwrap_or_default())
.collect();
let storage = crate::storage::MemoryFs::new();
let path = std::path::Path::new("out.csv");
crate::persistence::export_csv_to(&storage, &app.model_state.workbook, "Default", path)
.expect("the export writes");
let bytes = crate::storage::Storage::read(&storage, path).unwrap();
let csv = String::from_utf8(bytes).unwrap();
let exported: Vec<String> = csv
.lines()
.find_map(|l| l.strip_prefix("Q1,"))
.expect("a Q1 line")
.split(',')
.map(String::from)
.collect();
assert_eq!(
drawn,
["40.00", "8.00", "0.80"],
"March is hidden, so not folded"
);
assert_eq!(exported, drawn, "the export and the grid disagree:\n{csv}");
}
}
/// ADR 001's basis check, both arms per tier (improvise-xwi6/fkfg). The fixtures
+12 -10
View File
@@ -135,18 +135,20 @@ pub fn apply_command(app: &mut App, cmd: Checked<'_>) -> Dirty {
/// was derived from (improvise-kmgs). Exhaustive on purpose.
pub fn model_effects(app: &App, cmd: &ModelCommand) -> Vec<Box<dyn Effect>> {
let one: Box<dyn Effect> = match cmd {
// A cell the host projects as no `Record` is refused, so the frame is
// Rejected rather than written (improvise-ciw8, ADR 002).
ModelCommand::SetCell { cell, value } => {
let Some(key) = app.resolve_cell(cell) else {
return vec![];
};
Box::new(effect::SetCell(key, value.clone()))
}
ModelCommand::ClearCell(cell) => {
let Some(key) = app.resolve_cell(cell) else {
return vec![];
};
Box::new(effect::ClearCell(key))
match (app.unwritable(cell), app.resolve_cell(cell)) {
(Some(why), _) => effect::refuse(why),
(None, None) => return vec![],
(None, Some(key)) => Box::new(effect::SetCell(key, value.clone())),
}
}
ModelCommand::ClearCell(cell) => match (app.unwritable(cell), app.resolve_cell(cell)) {
(Some(why), _) => effect::refuse(why),
(None, None) => return vec![],
(None, Some(key)) => Box::new(effect::ClearCell(key)),
},
ModelCommand::AddItem { category, item } => Box::new(effect::AddItem {
category: category.clone(),
item: item.clone(),
+2 -4
View File
@@ -6,8 +6,8 @@ use unicode_width::UnicodeWidthStr;
use improvise_protocol::header_level_widths;
use crate::format::{FormatPlan, NumberFormat};
use crate::model::Model;
use crate::model::cell::{CellKey, CellValue};
use crate::model::{Aggregation, Model};
use crate::view::{AxisEntry, GridLayout};
/// Minimum column width — enough for short numbers/labels + 1 char gap.
@@ -110,9 +110,7 @@ fn cell_value(
true => (None, layout.records_value(row, col)),
false => {
let key = layout.cell_key(row, col);
let value = key
.as_ref()
.and_then(|k| model.evaluate_aggregated(k, Aggregation::of(&layout.none_cats)));
let value = key.as_ref().and_then(|k| layout.evaluate(model, k));
(key, value)
}
}
+326 -67
View File
@@ -51,17 +51,50 @@ fn member_records(model: &Model, partial: &CellKey) -> Vec<CellKey> {
seen.into_iter().collect()
}
/// A formula-level aggregate over the data matching `context` minus the
/// target. Deliberately *not* [`apply_agg`]'s emptiness rule — §2.
/// A formula-level aggregate over the data matching each of `contexts` minus the
/// target, pooled. Deliberately *not* [`apply_agg`]'s emptiness rule — §2.
fn eval_agg(
model: &Model,
context: &CellKey,
contexts: &[CellKey],
target_category: &str,
func: &AggFunc,
inner: &Expr,
agg_filter: Option<&Filter>,
eval_at: &dyn Fn(&Expr, &CellKey) -> Result<f64, String>,
) -> Result<f64, String> {
// A filter overrides its coordinate, so members it rewrites to one item name
// one record set: pool each distinct context once (core.md §14).
let distinct: IndexSet<CellKey> = contexts
.iter()
.map(|c| match agg_filter {
Some(f) => c.clone().with(&f.category, &f.item),
None => c.clone(),
})
.collect();
let values = distinct
.iter()
.map(|c| agg_values(model, c, target_category, inner, agg_filter, eval_at))
.collect::<Result<Vec<_>, String>>()?
.concat();
match func {
AggFunc::Sum => Ok(values.iter().sum()),
AggFunc::Count => Ok(values.len() as f64),
AggFunc::Avg | AggFunc::Min | AggFunc::Max => {
apply_agg(&values, func).ok_or_else(|| "empty".to_string())
}
}
}
/// The values an aggregate folds at one context: a stored ref's cells, or the
/// expression asked of each member record — §13.
fn agg_values(
model: &Model,
context: &CellKey,
target_category: &str,
inner: &Expr,
agg_filter: Option<&Filter>,
eval_at: &dyn Fn(&Expr, &CellKey) -> Result<f64, String>,
) -> Result<Vec<f64>, String> {
let mut partial = context.without(target_category);
// The fast path is the fold specialised to a ref that names a STORED
// cell; where it does not, the premise fails — §13, s08t, s2w3.
@@ -83,15 +116,15 @@ fn eval_agg(
if let Some(f) = agg_filter {
partial = partial.with(&f.category, &f.item);
}
let values: Vec<f64> = match select_raw {
match select_raw {
// A bare ref asks for the values that exist; anything else asks the
// expression of each member record and folds the answers — §13.
true => model
true => Ok(model
.data
.matching_values(&partial.0)
.into_iter()
.filter_map(|v| v.as_f64())
.collect(),
.collect()),
false => member_records(model, &partial)
.into_iter()
.map(|record| {
@@ -101,28 +134,48 @@ fn eval_agg(
.fold(partial.clone(), |k, (cat, item)| k.with(cat, item));
eval_at(inner, &at)
})
.collect::<Result<Vec<f64>, String>>()?,
};
match func {
AggFunc::Sum => Ok(values.iter().sum()),
AggFunc::Count => Ok(values.len() as f64),
AggFunc::Avg | AggFunc::Min | AggFunc::Max => {
apply_agg(&values, func).ok_or_else(|| "empty".to_string())
}
.collect(),
}
}
// How many formulas a fold has evaluated: what improvise-o1y0's regression counts.
#[cfg(test)]
thread_local! {
static FOLD_FORMULA_EVALUATIONS: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
}
/// One fold evaluation's memory: each formula key is evaluated once, and a key met
/// while still in progress (`None`) is a cycle (improvise-o1y0).
type FoldMemo = std::cell::RefCell<HashMap<CellKey, Option<Option<CellValue>>>>;
/// Which members of a collapsed group its fold covers. The model knows no
/// view, so the grid passes the members it draws (core.md §14).
pub type Covers<'a> = &'a dyn Fn(&str, &str) -> bool;
/// Where a formula's references are read: at one record, or across the members
/// a collapsed group's fold covers, as a None axis reads them (core.md §14).
#[derive(Clone, Copy)]
enum Reach<'a> {
Record,
Fold {
agg: Aggregation,
covers: Covers<'a>,
memo: &'a FoldMemo,
},
}
/// The **record-level** evaluator: no cache, because a member key is not a stem
/// the cache holds. Errors propagate immediately — §3, §13.
/// the cache holds. Errors propagate immediately — §3, §13. `reach`: §14.
fn eval_expr(
expr: &Expr,
context: &CellKey,
model: &Model,
target_category: &str,
depth: u8,
reach: Reach,
) -> Result<f64, String> {
use crate::formula::BinOp;
let eval = |e: &Expr| eval_expr(e, context, model, target_category, depth, reach);
match expr {
Expr::Number(n) => Ok(*n),
Expr::Ref(name) => {
@@ -130,15 +183,21 @@ fn eval_expr(
.find_item_category(name)
.ok_or_else(|| format!("ref:{name}"))?;
let new_key = context.clone().with(cat, name);
match model.evaluate_depth(&new_key, depth) {
let value = match reach {
Reach::Record => model.evaluate_depth(&new_key, depth),
Reach::Fold { agg, covers, memo } => {
model.evaluate_fold(&new_key, agg, covers, memo)
}
};
match value {
Some(CellValue::Number(n)) => Ok(n),
Some(CellValue::Error(e)) => Err(e),
_ => Err(format!("ref:{name}")),
}
}
Expr::BinOp(op, l, r) => {
let lv = eval_expr(l, context, model, target_category, depth)?;
let rv = eval_expr(r, context, model, target_category, depth)?;
let lv = eval(l)?;
let rv = eval(r)?;
match op {
BinOp::Add => Ok(lv + rv),
BinOp::Sub => Ok(lv - rv),
@@ -153,22 +212,26 @@ fn eval_expr(
}
}
}
Expr::UnaryMinus(e) => Ok(-eval_expr(e, context, model, target_category, depth)?),
Expr::UnaryMinus(e) => Ok(-eval(e)?),
// A fold's aggregate pools every member record it covers, never the
// members' own aggregates (§14).
Expr::Agg(func, inner, agg_filter) => eval_agg(
model,
context,
&model.contexts_of(context, reach),
target_category,
func,
inner.as_ref(),
agg_filter.as_ref(),
&|e, at| eval_expr(e, at, model, target_category, depth),
&|e, at| eval_expr(e, at, model, target_category, depth, Reach::Record),
),
Expr::Is(filter) => Ok(eval_is(context, filter)),
Expr::Is(filter) => Ok(eval_is(
model.read_coordinate(context, &filter.category, reach),
filter,
)),
Expr::If(cond, then, else_) => {
let cv = eval_bool(cond, context, model, target_category, depth)?;
match cv {
true => eval_expr(then, context, model, target_category, depth),
false => eval_expr(else_, context, model, target_category, depth),
match eval_bool(cond, context, model, target_category, depth, reach)? {
true => eval(then),
false => eval(else_),
}
}
}
@@ -182,12 +245,13 @@ fn eval_bool(
model: &Model,
target_category: &str,
depth: u8,
reach: Reach,
) -> Result<bool, String> {
use crate::formula::BinOp;
match expr {
Expr::BinOp(op, l, r) => {
let lv = eval_expr(l, context, model, target_category, depth)?;
let rv = eval_expr(r, context, model, target_category, depth)?;
let lv = eval_expr(l, context, model, target_category, depth, reach)?;
let rv = eval_expr(r, context, model, target_category, depth, reach)?;
match op {
BinOp::Eq => Ok((lv - rv).abs() < FLOAT_EQ_EPSILON),
BinOp::Ne => Ok((lv - rv).abs() >= FLOAT_EQ_EPSILON),
@@ -211,10 +275,8 @@ fn eval_bool(
/// `Cat IS Item` as a number. A cell with no `Cat` coordinate is **not** that
/// item, matching `Formula::filter`'s own `unwrap_or(false)` — §13.
fn eval_is(context: &CellKey, filter: &Filter) -> f64 {
context
.get(&filter.category)
.is_some_and(|v| v == filter.item.as_str()) as i32 as f64
fn eval_is(coordinate: Option<&str>, filter: &Filter) -> f64 {
coordinate.is_some_and(|v| v == filter.item.as_str()) as i32 as f64
}
/// Whether a grid hides dimensions a cell key does not name. The emptiness of
@@ -523,33 +585,88 @@ impl Model {
}
pub fn evaluate_aggregated(&self, key: &CellKey, agg: Aggregation) -> Option<CellValue> {
let members = self.group_expansion(key);
if !members.is_empty() {
return self.evaluate_group(key, &members, agg);
self.evaluate_covering(key, agg, &|_, _| true)
}
/// [`Self::evaluate_aggregated`], with a collapsed group's fold covering only
/// the members `covers` admits — the ones a grid draws (core.md §14).
pub fn evaluate_covering(
&self,
key: &CellKey,
agg: Aggregation,
covers: Covers,
) -> Option<CellValue> {
self.evaluate_fold(key, agg, covers, &FoldMemo::default())
}
/// A key naming a group folds; any other key reads as before. A formula is
/// evaluated OVER the fold, never folded itself — why: core.md §14.
fn evaluate_fold(
&self,
key: &CellKey,
agg: Aggregation,
covers: Covers,
memo: &FoldMemo,
) -> Option<CellValue> {
let members = self.group_expansion(key, covers);
match (members.is_empty(), self.formula_for(key)) {
(true, _) => self.evaluate_unfolded(key, agg),
(false, Some(formula)) => {
self.fold_formula_once(formula, key, &members, (agg, covers, memo))
}
(false, None) => self.fold_stored(key, &members, agg),
}
}
/// A formula at a group key, evaluated at most once per fold evaluation; a key
/// already in progress is a cycle, so `circular` still comes out (improvise-o1y0).
fn fold_formula_once(
&self,
formula: &Formula,
key: &CellKey,
members: &[CellKey],
(agg, covers, memo): (Aggregation, Covers, &FoldMemo),
) -> Option<CellValue> {
let seen = memo.borrow().get(key).cloned();
match seen {
Some(Some(value)) => value,
Some(None) => Some(CellValue::Error("circular".into())),
None => {
memo.borrow_mut().insert(key.clone(), None);
#[cfg(test)]
FOLD_FORMULA_EVALUATIONS.with(|n| n.set(n.get() + 1));
let reach = Reach::Fold { agg, covers, memo };
let value = self.eval_formula_folded(formula, key, members, agg, reach);
memo.borrow_mut().insert(key.clone(), Some(value.clone()));
value
}
}
}
/// A key that names no group: exact, the formula cache, or raw aggregation.
fn evaluate_unfolded(&self, key: &CellKey, agg: Aggregation) -> Option<CellValue> {
if agg == Aggregation::Exact {
return self.evaluate(key);
}
// Check the formula cache first.
if let Some(cached) = self.formula_cache.get(key) {
return Some(cached.clone());
}
// Aggregate raw data across all None-axis categories.
aggregate_cell_values(&self.data.matching_values(&key.0), self.agg_for(key))
}
/// One key per member where `key` names a collapsed GROUP rather than an item —
/// the cross product over every such coordinate. Empty when none does (§14).
fn group_expansion(&self, key: &CellKey) -> Vec<CellKey> {
/// One key per covered member where `key` names a collapsed GROUP rather than
/// an item — the cross product over every such coordinate. Empty when none does.
fn group_expansion(&self, key: &CellKey, covers: Covers) -> Vec<CellKey> {
let mut expanded = vec![key.clone()];
for (cat_name, name) in &key.0 {
let members = self
let members: Vec<&str> = self
.category(cat_name)
.filter(|cat| !cat.items.contains_key(name.as_str()))
.map(|cat| cat.items_in_group(name))
.unwrap_or_default();
.unwrap_or_default()
.into_iter()
.filter(|m| covers(cat_name, m))
.collect();
if members.is_empty() {
continue;
}
@@ -564,29 +681,85 @@ impl Model {
}
}
/// A group's cell: stored values fold ONCE over every record the group covers,
/// a derived one folds its members' results — why the two differ: core.md §14.
fn evaluate_group(
/// A coordinate as a formula reads it. At a fold a group's name is no item, so
/// the folded category reads as absent, as on `Axis::None` (improvise-lnkb).
fn read_coordinate<'k>(
&self,
context: &'k CellKey,
cat: &str,
reach: Reach,
) -> Option<&'k str> {
let value = context.get(cat)?;
match reach {
Reach::Fold { .. } if self.names_a_group(cat, value) => None,
Reach::Fold { .. } | Reach::Record => Some(value),
}
}
/// Whether `name` is a group of `cat` rather than one of its items.
fn names_a_group(&self, cat: &str, name: &str) -> bool {
self.category(cat)
.is_some_and(|c| !c.items.contains_key(name) && !c.items_in_group(name).is_empty())
}
/// The contexts an aggregate inside a formula pools over at `context`.
fn contexts_of(&self, context: &CellKey, reach: Reach) -> Vec<CellKey> {
let members = match reach {
Reach::Record => Vec::new(),
Reach::Fold { covers, .. } => self.group_expansion(context, covers),
};
match members.is_empty() {
true => vec![context.clone()],
false => members,
}
}
/// A group's stored values fold ONCE over every record the group covers.
fn fold_stored(
&self,
key: &CellKey,
members: &[CellKey],
agg: Aggregation,
) -> Option<CellValue> {
let values: Vec<CellValue> = match self.formula_for(key) {
Some(_) => members
.iter()
.filter_map(|k| self.evaluate_aggregated(k, agg))
.collect(),
None => members
.iter()
.flat_map(|k| match agg {
Aggregation::Exact => self.data.get(k).into_iter().collect(),
Aggregation::OverHidden => self.data.matching_values(&k.0),
})
.cloned()
.collect(),
};
aggregate_cell_values(&values.iter().collect::<Vec<_>>(), self.agg_for(key))
let values: Vec<&CellValue> = members
.iter()
.flat_map(|k| match agg {
Aggregation::Exact => self.data.get(k).into_iter().collect(),
Aggregation::OverHidden => self.data.matching_values(&k.0),
})
.collect();
aggregate_cell_values(&values, self.agg_for(key))
}
/// A formula at a group key, its refs read through the fold — the value the
/// same formula has with the group's category on `Axis::None` (§14).
fn eval_formula_folded(
&self,
formula: &Formula,
key: &CellKey,
members: &[CellKey],
agg: Aggregation,
reach: Reach,
) -> Option<CellValue> {
let applies = formula
.filter
.as_ref()
.is_none_or(|f| self.read_coordinate(key, &f.category, reach) == Some(f.item.as_str()));
if !applies {
return self.fold_stored(key, members, agg);
}
let depth = Self::MAX_EVAL_DEPTH;
match eval_expr(
&formula.expr,
key,
self,
&formula.target_category,
depth,
reach,
) {
Ok(n) => Some(CellValue::Number(n)),
Err(e) => Some(CellValue::Error(e)),
}
}
/// [`Self::evaluate_aggregated`] as `f64`, with `0.0` for an empty cell.
@@ -744,16 +917,19 @@ impl Model {
)?),
Expr::Agg(func, inner, agg_filter) => eval_agg(
model,
context,
std::slice::from_ref(context),
target_category,
func,
inner.as_ref(),
agg_filter.as_ref(),
// The fold evaluates at a MEMBER key, which is not a stem the
// cache holds — so the record-level evaluator, in both paths.
&|e, at| eval_expr(e, at, model, target_category, Model::MAX_EVAL_DEPTH),
&|e, at| {
let depth = Model::MAX_EVAL_DEPTH;
eval_expr(e, at, model, target_category, depth, Reach::Record)
},
),
Expr::Is(filter) => Ok(eval_is(context, filter)),
Expr::Is(filter) => Ok(eval_is(context.get(&filter.category), filter)),
Expr::If(cond, then, else_) => {
let cv = eval_bool_cached(cond, context, model, target_category, none_cats)?;
if cv {
@@ -841,6 +1017,7 @@ impl Model {
self,
&formula.target_category,
depth,
Reach::Record,
) {
Ok(n) => Some(CellValue::Number(n)),
Err(e) => Some(CellValue::Error(e)),
@@ -891,7 +1068,17 @@ mod agg_helper_tests {
let empty = Expr::Ref("Revenue".to_string());
let ctx = CellKey::new(vec![]);
let never = |_: &Expr, _: &CellKey| unreachable!("a bare Ref never folds per record");
let run = |func| eval_agg(&model, &ctx, "_Measure", &func, &empty, None, &never);
let run = |func| {
eval_agg(
&model,
std::slice::from_ref(&ctx),
"_Measure",
&func,
&empty,
None,
&never,
)
};
assert_eq!(run(AggFunc::Sum), Ok(0.0));
assert_eq!(run(AggFunc::Count), Ok(0.0));
@@ -1493,6 +1680,78 @@ mod model_tests {
);
}
/// improvise-o1y0: a fold evaluates each formula it reaches once — see
/// context/test-evidence/core.md.
#[test]
fn a_fold_evaluates_each_formula_it_reaches_once() {
let mut m = quarter_model(&[("Jan", "A", 3.0), ("Feb", "A", 4.0)]);
m.add_formula(crate::formula::parse_formula("F0 = Revenue", "_Measure").unwrap());
for i in 1..=8 {
let text = format!("F{i} = F{p} + F{p}", p = i - 1);
m.add_formula(crate::formula::parse_formula(&text, "_Measure").unwrap());
}
let key = coord(&[("Month", "Q1"), ("Store", "A"), ("_Measure", "F8")]);
super::FOLD_FORMULA_EVALUATIONS.with(|n| n.set(0));
let value = m.evaluate_covering(&key, Aggregation::Exact, &|_, _| true);
let evaluations = super::FOLD_FORMULA_EVALUATIONS.with(|n| n.get());
assert_eq!(
value,
Some(CellValue::Number(7.0 * 256.0)),
"the chain's value"
);
assert_eq!(evaluations, 9, "F0..F8 once each, not 2^9 - 1");
}
/// improvise-o1y0's other half: a key met while still in progress is a cycle.
#[test]
fn a_cycle_at_a_fold_is_still_circular() {
let mut m = quarter_model(&[("Jan", "A", 3.0), ("Feb", "A", 4.0)]);
m.add_formula(crate::formula::parse_formula("P = Q + 1", "_Measure").unwrap());
m.add_formula(crate::formula::parse_formula("Q = P + 1", "_Measure").unwrap());
let key = coord(&[("Month", "Q1"), ("Store", "A"), ("_Measure", "P")]);
assert_eq!(
m.evaluate_covering(&key, Aggregation::Exact, &|_, _| true),
Some(CellValue::Error("circular".into()))
);
}
/// improvise-d7aj: the memo is per KEY, not per formula — one formula met at two
/// fold keys in one evaluation. Evidence: context/test-evidence/core.md.
#[test]
fn one_formula_at_two_fold_keys_is_evaluated_at_each() {
let mut m = quarter_model(&[
("Jan", "A", 3.0),
("Feb", "A", 4.0),
("Jan", "B", 100.0),
("Feb", "B", 200.0),
]);
for text in ["G = Revenue * 2", "F = IF(Revenue > 50, G, G + B)"] {
m.add_formula(crate::formula::parse_formula(text, "_Measure").unwrap());
}
let key = coord(&[("Month", "Q1"), ("Store", "A"), ("_Measure", "F")]);
assert_eq!(
m.evaluate_covering(&key, Aggregation::Exact, &|_, _| true),
Some(CellValue::Number(614.0)),
"G at A (14) plus F at B, which is G at B (600)"
);
}
/// improvise-d7aj: a formula reached twice by two paths is finished, not in
/// progress, the second time. Evidence: context/test-evidence/core.md.
#[test]
fn a_diamond_at_a_fold_is_not_circular() {
let mut m = quarter_model(&[("Jan", "A", 3.0), ("Feb", "A", 4.0)]);
for text in ["A1 = B2 + C3", "B2 = D * 2", "C3 = D * 3", "D = Revenue"] {
m.add_formula(crate::formula::parse_formula(text, "_Measure").unwrap());
}
let key = coord(&[("Month", "Q1"), ("Store", "A"), ("_Measure", "A1")]);
super::FOLD_FORMULA_EVALUATIONS.with(|n| n.set(0));
let value = m.evaluate_covering(&key, Aggregation::Exact, &|_, _| true);
let evaluations = super::FOLD_FORMULA_EVALUATIONS.with(|n| n.get());
assert_eq!(value, Some(CellValue::Number(35.0)), "14 + 21 over D = 7");
assert_eq!(evaluations, 4, "A1, B2, C3 and D once each");
}
/// Evidence: context/test-evidence/core.md.
#[test]
fn a_groups_stored_values_fold_once_not_per_member() {
+189 -3
View File
@@ -1,3 +1,4 @@
use std::collections::{HashMap, HashSet};
use std::rc::Rc;
use crate::model::category::{DIM_CATEGORY, INDEX_CATEGORY, MEASURE_CATEGORY};
@@ -377,6 +378,8 @@ pub struct GridLayout {
/// In records mode: what each column holds, parallel to `col_items`.
/// Empty for normal pivot views.
record_cols: Vec<RecordCol>,
/// The view's hidden items: a fold covers only the members this grid draws.
hidden: HashMap<String, HashSet<String>>,
}
impl GridLayout {
@@ -460,6 +463,7 @@ impl GridLayout {
none_cats,
records: None,
record_cols: Vec::new(),
hidden: view.hidden_items.clone(),
}
}
}
@@ -493,6 +497,7 @@ impl GridLayout {
none_cats,
records: Some(Rc::new(records)),
record_cols,
hidden: HashMap::new(),
}
}
@@ -611,7 +616,7 @@ impl GridLayout {
for (ci, cell) in row.iter_mut().enumerate() {
*cell = self
.cell_key(ri, ci)
.and_then(|k| model.evaluate_aggregated(&k, Aggregation::of(&self.none_cats)))
.and_then(|k| self.evaluate(model, &k))
.is_some();
}
}
@@ -649,6 +654,13 @@ impl GridLayout {
axis_label(&self.col_items, col)
}
/// The value of a pivot key as THIS grid reads it: aggregated over its None
/// axis, and a fold over the members it draws (core.md §14). One evaluation.
pub fn evaluate(&self, model: &Model, key: &CellKey) -> Option<CellValue> {
let drawn = |cat: &str, item: &str| !self.hidden.get(cat).is_some_and(|h| h.contains(item));
model.evaluate_covering(key, Aggregation::of(&self.none_cats), &drawn)
}
/// Display text for (row, col) in either mode.
pub fn display_text(
&self,
@@ -662,8 +674,7 @@ impl GridLayout {
} else {
self.cell_key(row, col)
.and_then(|key| {
let value =
model.evaluate_aggregated(&key, Aggregation::of(&self.none_cats))?;
let value = self.evaluate(model, &key)?;
Some(plan.at(Some(&key)).value(Some(&value)))
})
.unwrap_or_default()
@@ -1780,6 +1791,181 @@ mod tests {
);
}
/// Q2 = {Apr}, Q1 = {Jan, Feb}, so the Q1 fold is row 1. Jan and Feb carry
/// ratios 0.5 and 0.9 whose sum, mean and pooled ratio all differ; Apr is empty.
fn margin_book(formulas: &[&str]) -> Workbook {
let mut wb = Workbook::new("T");
wb.add_category("Month").unwrap();
wb.add_item_in_group("Month", "Apr", "Q2").unwrap();
wb.add_item_in_group("Month", "Jan", "Q1").unwrap();
wb.add_item_in_group("Month", "Feb", "Q1").unwrap();
wb.add_item("_Measure", "Revenue").unwrap();
wb.add_item("_Measure", "Cost").unwrap();
for (month, revenue, cost) in [("Jan", 10.0, 5.0), ("Feb", 30.0, 3.0)] {
for (measure, n) in [("Revenue", revenue), ("Cost", cost)] {
wb.model.set_cell(
CellKey::new(vec![
("Month".into(), month.into()),
("_Measure".into(), measure.into()),
]),
CellValue::Number(n),
);
}
}
for f in formulas {
wb.model
.add_formula(crate::formula::parse_formula(f, "_Measure").unwrap());
}
wb.active_view_mut().set_axis("Month", Axis::Row);
wb.active_view_mut().set_axis("_Measure", Axis::Column);
wb
}
/// `measure` at the row labelled `row_label`, as the grid reads it.
fn grid_value(wb: &mut Workbook, row_label: &str, measure: &str) -> Option<f64> {
wb.model.recompute_formulas(&wb.active_view().none_cats());
let layout = GridLayout::new(&wb.model, wb.active_view());
let row = (0..layout.row_count()).find(|&r| layout.row_label(r) == row_label)?;
let col = (0..layout.col_count()).find(|&c| layout.col_label(c) == measure)?;
layout
.evaluate(&wb.model, &layout.cell_key(row, col)?)
.and_then(|v| v.as_f64())
}
/// improvise-eccl — see context/test-evidence/core.md.
#[test]
fn a_folds_ratio_formula_is_the_ratio_of_its_folded_inputs() {
for formulas in [
&["Margin = (Revenue - Cost) / Revenue"][..],
&["Profit = Revenue - Cost", "Margin = Profit / Revenue"][..],
] {
let mut folded = margin_book(formulas);
folded
.active_view_mut()
.toggle_group_collapse("Month", "Q1");
let fold = grid_value(&mut folded, "Q1", "Margin").expect("a Q1 fold");
let mut none = margin_book(formulas);
none.active_view_mut().set_axis("Month", Axis::None);
let over_none = grid_value(&mut none, "", "Margin").expect("a None-axis cell");
assert!((fold - 0.8).abs() < 1e-9, "{formulas:?}: fold {fold}");
assert!(
(fold - over_none).abs() < 1e-9,
"{formulas:?}: {fold} vs {over_none}"
);
}
}
/// improvise-lnkb: the folded category reads as absent, as on `Axis::None` — see
/// context/test-evidence/core.md.
#[test]
fn a_fold_answers_is_and_where_on_its_category_as_the_none_axis_does() {
let formulas = [
"Flag = Month IS \"Q1\"",
"Marked = Revenue * 2 WHERE Month = \"Q1\"",
];
for measure in ["Flag", "Marked"] {
let mut folded = margin_book(&formulas);
folded
.active_view_mut()
.toggle_group_collapse("Month", "Q1");
let fold = grid_value(&mut folded, "Q1", measure);
let mut none = margin_book(&formulas);
none.active_view_mut().set_axis("Month", Axis::None);
let over_none = grid_value(&mut none, "", measure);
assert_eq!(
fold, over_none,
"{measure}: the fold and the None axis differ"
);
}
let mut folded = margin_book(&formulas);
folded
.active_view_mut()
.toggle_group_collapse("Month", "Q1");
assert_eq!(
grid_value(&mut folded, "Q1", "Flag"),
Some(0.0),
"false, not absent"
);
}
/// improvise-eccl's guard on the other side: an aggregate INSIDE the formula
/// still pools every record the fold covers.
#[test]
fn a_folds_aggregate_formula_pools_its_members_records() {
let mut wb = margin_book(&["Top = MAX(Revenue)", "Total = SUM(Revenue)"]);
wb.active_view_mut().toggle_group_collapse("Month", "Q1");
assert_eq!(grid_value(&mut wb, "Q1", "Total"), Some(40.0));
assert_eq!(grid_value(&mut wb, "Q1", "Top"), Some(30.0));
}
/// An inline `WHERE` on the folded category makes every member context the
/// same record set — see context/test-evidence/core.md.
#[test]
fn a_folds_aggregate_filtered_on_the_folded_category_counts_each_record_once() {
let formula = ["Jans = SUM(Revenue WHERE Month = \"Jan\")"];
let mut folded = margin_book(&formula);
folded
.active_view_mut()
.toggle_group_collapse("Month", "Q1");
let fold = grid_value(&mut folded, "Q1", "Jans");
let mut none = margin_book(&formula);
none.active_view_mut().set_axis("Month", Axis::None);
assert_eq!(fold, Some(10.0), "Jan's revenue, once");
assert_eq!(
fold,
grid_value(&mut none, "", "Jans"),
"the None-axis answer"
);
}
/// Every row's value in column 0, summed: the totals strip's arithmetic.
fn column_total(wb: &Workbook) -> f64 {
let layout = GridLayout::new(&wb.model, wb.active_view());
(0..layout.row_count())
.filter_map(|r| layout.evaluate(&wb.model, &layout.cell_key(r, 0)?))
.filter_map(|v| v.as_f64())
.sum()
}
/// improvise-f2oo — see context/test-evidence/core.md.
#[test]
fn collapsing_a_group_with_a_hidden_member_leaves_the_total_where_it_was() {
let mut wb = Workbook::new("T");
wb.add_category("Month").unwrap();
wb.add_category("Type").unwrap();
wb.add_item("Type", "Food").unwrap();
for (month, quarter, n) in [("Jan", "Q1", 3.0), ("Feb", "Q1", 4.0), ("Apr", "Q2", 100.0)] {
wb.add_item_in_group("Month", month, quarter).unwrap();
wb.model.set_cell(
CellKey::new(vec![
("Month".into(), month.into()),
("Type".into(), "Food".into()),
]),
CellValue::Number(n),
);
}
wb.active_view_mut().set_axis("Month", Axis::Row);
wb.active_view_mut().set_axis("Type", Axis::Column);
wb.active_view_mut().hide_item("Month", "Feb");
assert_eq!(column_total(&wb), 103.0, "expanded: Jan and Apr are drawn");
wb.active_view_mut().toggle_group_collapse("Month", "Q1");
assert_eq!(column_total(&wb), 103.0, "z moved the total");
assert_eq!(
grid_value(&mut wb, "Q1", "Food"),
Some(3.0),
"Feb is hidden"
);
// Every member hidden: no fold is drawn, and the total still agrees.
wb.active_view_mut().hide_item("Month", "Jan");
assert_eq!(grid_value(&mut wb, "Q1", "Food"), None, "no Q1 row");
assert_eq!(column_total(&wb), 100.0);
}
/// improvise-t0ma: the steps themselves, not their translation into effects —
/// see context/test-evidence/core.md.
#[test]