Files
improvise/context/code/core.md
T
Edward Langley e44b1491ca Merge gh/main (cc95cfbf) into fix/e265-levelled-groups
# Conflicts:
#	context/test-evidence/command.md
#	context/test-evidence/core.md
2026-09-23 23:26:55 -07:00

84 KiB
Raw Blame History

improvise-core — the arguments

Written for a reader with the code open. This is where the reasoning that used to live in crates/improvise-core/**'s comments now lives; the comments themselves are capped at two lines (design-principles §6). Bug narratives keyed by test name are in context/test-evidence/core.md.

The crate is pure data: Model, View, GridLayout, Workbook, number formatting. It depends on improvise-formula for AST types and knows nothing about UI, IO or commands.


1. CellKey is canonical by construction

CellKey(Vec<(category, item)>) is always sorted by category name, and the constructors are the only way to get one: CellKey::new(coords) sorts, with(cat, item) replaces-or-inserts-and-re-sorts, without(cat) filters and so preserves the order. Equality, hashing and storage all rely on the canonical form, so two keys naming the same intersection in different orders are the same key. Never build the inner Vec directly (gotcha 5).

That without preserves the sort is what lets record_of be a one-liner rather than a normalisation step — see §5.

2. Aggregating a cell's raw values

Three functions, deliberately with three different emptiness rules, and the differences are the whole content of the design (improvise-kdp):

  • apply_agg(values: &[f64], func) — reduces numbers. None on empty: an empty cell reads as absent, not as zero, or every blank intersection in a pivot would render 0. Both aggregation paths (evaluate_aggregated and aggregate_raw) go through it so their semantics cannot drift.
  • aggregate_cell_values(values: &[&CellValue], func) — reduces a mixed set. See below.
  • eval_agg(...) — evaluates a formula-level SUM(x) / AVG(x WHERE …). Its emptiness rule is not apply_agg's: an aggregate over no rows is 0 for SUM/COUNT, which have an identity, and an error for AVG/MIN/MAX, which do not.

evaluate_aggregated takes Aggregation, not a list it cannot read

The parameter was none_cats: &[String], and the body consulted it exactly once, for emptiness — a boolean wearing a list's type, since the aggregation itself runs over key.0 through matching_values. It never filtered by that list, in any revision: git log -S back to the commit that introduced Axis::None shows the same is_empty() and nothing else. So the name promised a filter the body has never performed, and the next caller to build a careful list would have threaded it into a parameter that cannot use it.

Aggregation::{Exact, OverHidden} admits only the two states the body distinguishes, and Aggregation::of(none_cats) performs the reduction once, at the boundary that holds the list (improvise-5jm5, doctrine(an_invariant_is_made_unrepresentable_not_validated) — the same answer prune_axis_items and gp-tree's Row<T> got). It is an enum rather than a bool because the two states have names worth reading at the call site, and the regression guard is the signature: the thing the ticket describes — two different lists, same emptiness, same answer — is no longer constructible.

Note what does not change: recompute_formulas and aggregate_raw keep &[String], because they walk the elements (stripping each hidden coordinate off a stem). Two parameters that looked alike were doing two different jobs.

Non-numeric values reduce by unanimity, and the branch order is the rule

aggregate_cell_values checks the all-numeric branch first. If every value is a number it reduces through apply_agg; otherwise, if every value equals the first, the result is that value; otherwise it is CellValue::Error("mixed").

That ordering is the entire difference between this rule and "a single value passes through". Count is the only function that can tell them apart — every other function is the identity on a one-element set — so Count of a lone number must still be 1 and not the number itself. a_lone_number_still_counts_ as_one_rather_than_passing_through is the pin.

What the rule forbids is dropping. The old code did filter_map(as_f64), and since _Index/_Dim sit on Axis::None in every default view, none_cats is non-empty almost always and the aggregating path was almost always taken — so every CellValue::Text in every document rendered blank, including the pipe-quoted text .improv carries as a first-class value (Region=West, Measure=Revenue = |pending|). A user could type a text value, save it, load it, and never see it again (improvise-l4qx).

Both evaluate_aggregated and aggregate_raw share the helper, so a formula ref over mixed values now errors "mixed" rather than silently summing the numeric ones and reading as authoritative while under-reporting.

3. Formula evaluation

Fixed point. recompute_formulas(none_cats) clears formula_cache, collects the distinct coordinate stems (each data key with the formula's target category and every none_cat stripped), and iterates stem × formula target until nothing changes, bounded by MAX_EVAL_DEPTH = 16. Chained formulas (Margin = Profit / Revenue over Profit = Revenue - Cost) converge naturally; circular references converge to CellValue::Error("circular") rather than overflowing the stack.

A cell still moving when the bound runs out has not converged, and the cached path had to be told so (improvise-7iww). The direct evaluator gets this free: evaluate_depth bottoms out at depth == 0 and returns Error("circular"). The fixed point had no such arm — it simply stopped iterating and left the sixteenth iteration in formula_cache, which evaluate_aggregated then served as a valid number. Revenue = Revenue + 1 over a stored Revenue = 10 read Error("circular") directly and Number(26) through the cache, and the cached one is the displayed path, because the default views put the virtual categories on Axis::None. So a circular formula looked like a real calculated value. recompute_formulas now records the keys that changed on each pass and, if the last pass still changed any, writes Error("circular") over them.

Note what this costs and why it is the right cost: the fixed point can only confirm convergence with a pass that changes nothing, so a chain needing all sixteen passes is now reported circular. That is the same bound the direct evaluator already enforces — a sixteen-deep chain exhausts depth there too — so the two paths agree rather than diverge, which is the property §3 exists to protect. The anti-clamp guard is a_chained_formula_still_converges_after_the_circular_marker. The stems are an IndexSet: dedupe is O(1) per cell and insertion order is preserved (improvise-6os replaced an O(n²) Vec::contains).

There are two evaluators, and they must be changed together:

  • eval_formula_depth / eval_expr — the direct, uncached path, recursion bounded by depth.
  • eval_formula_with_cache / eval_expr_cached — the fixed-point path, which resolves a Expr::Ref through formula_cache first and falls back to aggregate_raw.

A rule stated in one and not the other is a live bug; improvise-0bf was exactly that (see §4).

find_item_category searches real categories first and then formula targets, because add_formula never inserts its target into _Measure (improvise-nrs). Without the fallback, a reference to a computed measure fails to resolve.

Display rounding never reaches eval. format_f64 is called only in rendering paths. Feeding a rounded value back into a formula is the bug formula_chain_preserves_full_precision exists to catch.

4. Float equality has one epsilon, and the div/0 guard shares it

FLOAT_EQ_EPSILON = 1e-10 is used by =/!= and by the division-by-zero guard, in both evaluators. The two must agree or IF(x = 0, a, y / x) is incoherent: the condition calls x zero and the else branch divides by it anyway. With x = 1e-11 the old exact rv == 0.0 guard returned 1e13 (improvise-0bf). 1e-10 is far below typical data magnitudes but absorbs accumulated f64 rounding noise from chained formulas.

Note the repo map's gotcha 17 (which called this "currently mixed") describes the pre-fix state; the guard and the comparison share the constant now.

5. Surrogate identities — model/record.rs (ADR 002)

A client sees a grid of label strings indexed by viewport position. It holds no Model, no category names and no page/filter constraints, so it cannot build a CellKey — which is why improvise-q08 found a thin client cannot compose SetCell. The answer is an opaque surrogate the server projects per visible cell and the client echoes back.

Two surrogates, because two kinds of thing are nameable and only one is settable. RecordHash names a record — the coordinate tuple minus _Measure (record_of). That unit is not chosen here: it is already ADR 001's unit of atomicity and already what the write counters are keyed by, because renaming a category coordinate in wide records mode moves every measure cell in the row (§6). One identity therefore serves addressing and staleness. AggregateHash names an aggregate's coordinate constraint — row and column coordinates plus the page and filter selections that decide which stored cells it sums (View::filtering_cats(), §7). An aggregate is not the absence of an identity; it is a different kind of identity, which is what makes Drill addressable at all. It is deliberately not normalised: a measure on Row or Column is part of what the aggregate sums, so dropping _Measure would give two visibly different totals one name.

The hash is specified here, not inherited. Three ways it breaks silently, each producing a wrong write rather than an error:

  1. Not DefaultHasher, not RandomState. std's default HashMap hasher is randomly seeded per process, so a surrogate would not survive a restart or agree between two processes. This is SipHash-2-4 under a constant key.
  2. Not #[derive(Hash)]. Rust gives no stability guarantee for a derived Hash across compiler versions or platforms. What is hashed is the explicit byte serialization in hash_coords, so the surrogate is defined by that format rather than by whatever the compiler did this week.
  3. 128 bits, argued rather than defaulted. A collision is a silent write to the wrong record — precisely what got _Index rejected as a surrogate — and no later check catches it: a colliding surrogate resolves successfully to a record the client never saw. 64 bits reaches birthday risk around 2^32 records, which a pivot tool will not see by live record count; but the risk is over every record a long-lived document ever names, and the consequence is unbounded and undetectable. The other choice costs 8 more bytes per visible cell — 8 KB on a full 50×20 projection and far less on the diffs that carry steady-state traffic. Bounded cost against silent corruption is not close.

Serialization details that are load-bearing. The byte format is tag ‖ count ‖ (len(category) ‖ category ‖ len(item) ‖ item)*, every length a little-endian u64. The lengths are what make it injective: without them ("ab","c") and ("a","bc") serialize alike. Only Hasher::write is used, never write_u64/write_u8, which are free to feed the hasher in native byte order. The SipHash key constants are written as big-endian readings of b"improvis"/b"e-record" so the constant is the same on every platform. A domain tag byte (0x01 record, 0x02 aggregate) is hashed first, because a record and an aggregate may be built from the same coordinates — an aggregate over a single record is the degenerate case — and they must not hash alike.

Hex on the wire, not a JSON number. serde_json writes a u128 as a bare number and a JavaScript host parses every JSON number as f64, so anything above 2^53 comes back changed — which for a surrogate means resolving to a different record or to none. The wire form is a fixed 32-character lowercase hex string.

6. Records mode is wide format (view/layout.rs)

A row is one coordinate tuple — the cells sharing everything but _Measure — and each raw measure gets its own editable column. The columns are [coordinates…, +cat, measures…, +]: one per regular category, then the column that creates a new category (improvise-2zq), then one per raw measure, then the column that creates a new measure (improvise-c5v). Each + sits adjacent to what it adds to; two trailing columns would have made the user read a label to tell them apart, and position says it instead. Formula targets are not columns — they are computed, so there is nothing to type into them.

Consequences: two measures on one coordinate are one row; editing a category column moves every measure cell in the row, not just the one under the cursor; and _Dim in the synthetic key carries the column index, not a label, because a category and a measure item may share a name and a label could not say which column an edit landed on. Resolve the index through GridLayout::record_col into a RecordCol. Drill pending edits are keyed by (row, RecordCol).

RecordRow::group collapses loose cells into wide rows in first-appearance order, so the result follows the store's order rather than a hash walk (§8). A cell with no _Measure coordinate still anchors a row — that is how add-record-row seeds one — but contributes no column value.

Records mode is a property of the view's axes, and View owns the one definition. View::is_records_layout() (_Index on Row and _Dim on Column) is what GridLayout::new asks and what on_category_added asks; two derivations would drift (improvise-edp, the disease this codebase keeps curing).

prune_axis_items takes a predicate, not a flag vector

prune_empty hands it impl Fn(usize) -> bool over the data index — the position a row/col argument means, which group headers do not consume. The predicate's domain is derived from items itself, so there is no parallel length a caller can get wrong and debug and release builds cannot disagree about an out-of-range flag. The earlier &[bool] + debug_assert pair could have, had a third caller ever sized keep from something other than row_count()/col_count() (improvise-n3wt, doctrine(an_invariant_is_made_unrepresentable_not_validated)).

A group header survives only when a data item that follows it survives. In prune_empty the row pass does not touch col_items or has_value, so the column pass still reads the original grid. Records mode is exempt entirely — the user drilled in to see all the raw data.

Group runs nest, and pruning re-marks rather than filters (improvise-e265)

On an axis with two grouped categories the runs nest: North in Upper holds Jan in Q1, and after Q1 ends North/Mar is still in Upper. So a marker names its level. GroupHeader { level, .. } opens a run at level and closes every deeper one; GroupEnd { level } closes level and every deeper one. expand_category reads the level off its prefix, which holds one item per outer category, so it is derived rather than passed. A row's group, for row_group_for/col_group_for and so for z, is the innermost run open at it. That is the user's ruling on the ticket: membership is per level, and z toggles the group nearest the cursor.

The marker used to carry no level, and every reader was flat: the nearest marker decided. Under a flat reading, pruning and membership could not both be right. Before #200 a pruned axis could draw a header over nothing; after it, a surviving row changed group when an inner header was pruned, because the inner GroupEnd went with it (45,806 of 300,580 enumerated cases, every one that shape). The unpruned answer was also lossy: North/Mar was in no group while it was drawn under the Upper header row.

Pruning therefore re-marks the survivors instead of filtering the markers. prune_axis_items tracks which run the input has open at each level (keyed by the header's position, so two runs under two equal headers stay two, as improvise-h0iv requires) and which run the kept entries have open. Before each surviving item, remark emits a header wherever a different run should be open and a GroupEnd wherever none should. Three properties follow by construction:

  • each survivor keeps exactly the runs it was in;
  • no header is emitted without a member, because headers are emitted only before an item;
  • no trailing GroupEnd, because a GroupEnd is also emitted only before an item.

This replaced the pending/close_runs_of/run_open machinery, which is where improvise-gd83, u6gc, lhi4 and b5zh each landed. When nothing is pruned the output is the input, marker for marker, so the exact-sequence tests from those tickets still pass unchanged. That equality needs one rule: remark closes the deeper runs innermost-out before it touches a level, as expand_category's trailing GroupEnds do. Without it the output is equivalent but not equal (End0 where the input has End1, End0), and the enumeration test asserts equality whenever every row survives.

How it is tested (improvise-6u6s). An enumeration compares each surviving row's whole open-run stack, and whether it shares a run with the previous survivor, pruned against unpruned. It runs over five fixtures, including a non-contiguous group, a level with no groups, and two adjacent outer groups. Without the non-contiguous fixture, a pruner that keyed runs by name instead of header position passed every test. open_runs is the one reading of the markers: innermost_group and the tests both use it. It is a lazy iterator, so a single lookup (z, which asks about one row) walks the axis only up to that row, rather than building and cloning a stack for every row of it (improvise-0r01).

A header's level also names its category (cats[level]) on the wire, where FrameGroup carries level and not the category (improvise-7xyx removed that field): see context/code/protocol.md §3a.

7. Axes: Page and Filter are two different questions

Axis is Row | Column | Page | Filter | None. Filter names a coordinate the drill fixed (improvise-dz8): like Page it constrains which records are visible, but unlike Page the user cannot navigate it. Axis::next() maps Filter to itself and no navigable axis rotates into it, so tile select cannot hand one out, and is_user_assignable() is false for it.

Picking the wrong question is the trap:

  • "what data is visible" → View::filtering_cats() (Page ∪ Filter). Used by GridLayout, by AggregateHash, and by add-record-row when it builds a key from the current constraints.
  • "what can the user act on" → categories_on(Axis::Page) for [/] page cycling, or View::tileable_cats() for all three tile-select sites.

Leaving the constraining axes forgets the page selection (improvise-z336)

set_axis used to leave page_selections alone; only on_category_removed and on_item_removed ever cleared one. So cycling a category off Page merely hid its filter, and cycling back to Page silently reinstated it — the same item, unasked. Measured in the browser: Pag(Espresso) → None → Row → Col → Page and Espresso is back.

That mattered more than it reads, because there is no clear-a-page-selection operation anywhere in this workspace — not in ModelCommand, not in the View API, and not in the TUI; [ and ] cycle among the category's items with no "All" to land on. So a user who had applied a filter had no way out of it at either front end, which is a product gap at parity rather than a browser regression. Forgetting the selection on the way out closes it with no new command, no new wire variant and no new key: cycling the axis round is now a way out.

The predicate is Axis::filters_data() — Page ∪ Filter — and not "is it Page". That is this section's own trap, seen from the writing side: a drill fixes its coordinates on Filter and .improv round-trips a selection under it, so clearing on everything-but-Page would pass a test written for the browser symptom and silently break drill.

Axis also owns four presentation vocabularies — plain_label(), glyph() (which Display appends, giving "Row ↕"), short_label() (three columns for the tile bar; None is "·", not a truncation of "None") and initial() (one letter for the view panel; None is empty) — plus color() -> AxisColor. AxisColor is a semantic palette because improvise-core must not name a ratatui type; draw.rs::axis_fg does the one mapping, exactly as minibuf_fg does for MinibufColor (improvise-rml). The tile bar and the category panel each carried a copy of the colour table before, with nothing keeping them in agreement.

on_category_added: a virtual on an axis is a configuration, not a vacancy

Virtual (underscore) categories seat on Axis::None. A regular category takes the first slot no regular category holds — Row, then Column, then Page.

This used to bump a virtual off Row or Column to make room, assuming a virtual sitting there was a placeholder. It never is, and the assumption cost two live bugs (improvise-2zq):

  • _Index on Row together with _Dim on Column is records mode, and the only production writers of those axes are the records and drill setups. Bumping _Index ejected the user from the grid they were standing in: naming a category with N in records mode silently returned them to a pivot.
  • _Measure on Column is an ordinary user configuration — it is what web/demo.improv ships. Bumping it made every measure vanish from the grid the moment a second regular category was added.

So a virtual is never moved. A records view seats the new category on Axis::None, which is where records mode keeps every regular category and where records_columns — which filters on kind.is_regular() and never looks at an axis — picks it up as a column immediately. Elsewhere the new category simply joins whatever virtual is already there, nesting a header rather than destroying one.

The negative arm is load-bearing and must stay tested: without a_pivot_view_still_seats_categories_row_then_column_then_page, "always seat on None" would pass every assertion about records mode. Probing confirmed the asymmetry — seating everything on None fails 12 tests, while removing the virtual bump failed none at all, which is how the bump came to be wrong in two directions with nothing noticing.

Known defect: two answers to "is this name a virtual"

on_category_added tests cat_name.starts_with('_') (twice) while is_records_layout() four lines away asks CategoryKind::of_name. That is improvise-bidq and it is open; do not assume either spelling is the intended one when editing near it.

8. Ordering is correctness, not tidiness (model/cell.rs)

DataStore holds cells in an IndexMap keyed by InternedKey, plus a secondary index HashMap<(Symbol, Symbol), HashSet<InternedKey>>.

An index whose values are HashSets is for O(1) lookup, never for iteration. matching_cells/matching_values used to walk the index's HashSet directly, so their order was hash order — different in every process, and different between two stores in one process, because RandomState keys each HashMap afresh. That defeated the whole reason cells is an IndexMap, and it was not tidiness (improvise-shyt). It caused, in ascending order of seriousness:

  • records-mode rows reordering between sessions;
  • SUM/AVG drifting in the last f64 bits, because f64 addition is not associative (Min/Max/Count and the text-unanimity rule of §2 are order-free, so they were never affected);
  • worst, the drill snapshot freezing its record list in hash order while drill_state.pending_edits are keyed by row index (§6) — so the same drill opened in a second process addressed different records. Silent misdirected writes, and nothing in the tracker knew.

matching_positions is the fix: the index still narrows the candidates, and the primary map orders them — each surviving candidate resolves to its position with one hash lookup and the positions are sorted. Shape stays O(smallest set × |partial|) plus O(m log m) over the matches, not a scan. Measured at 18,000 cells: +20% worst case on matching_values, and matching_cells 7% faster, sorted positions resolving CellKeys with better locality. The alternatives both move the cost — an ordered index value type pays on every set; scanning the primary is O(store) per query.

An empty partial matches every cell, vacuously, and matching_positions is where that is answered (improvise-hkt6). It used to index sets[0] and panic; both public callers happened to guard, so it was unreachable — but the guard lived one level away from the indexing, which is how this class of bug returns. The guards could not simply be pushed down as vec![]: "no coordinates constrain anything" means all positions, and answering "none" here would have made removing a caller's fast path change behaviour silently. The callers keep theirs as a fast path, not as a guard.

live_records has a decided order, not an inherited one (improvise-2yc2). It walks cells and projects insertion order onto records: a record sits where its first cell sits. It deliberately pays O(cells) rather than O(records), because cells' order is already the answer matching_positions gives and a second answer to "what order is this store in" is the disease. It is derived per call rather than memoised on RecordEntry, since a stored position is a copy and a record deleted and re-created would hold its old one while its cells sit at the end of the store. Note what is not on offer: an unordered return. Every iterator has an order, and a type that merely disclaims one still hands the caller hash order — the guard living one level away from the hazard, which is the shape this whole class of bug returns through. The function had no callers when this was decided, so the order was still free to decide rather than to discover; with no caller to benchmark against, correctness is the defensible default, and a hot-path caller can argue for a memoised position with numbers.

The record index: RecordEntry

DataStore::records: HashMap<RecordHash, RecordEntry> answers both of a client surrogate's questions — "which record do you mean?" (ADR 002) and "is your basis stale?" (ADR 001) — from one entry in one lookup, because both are keyed by the record and both are invalidated at the same moment.

It lives in the store because this is where the same job is already done: the store already interns every name and already maintains a secondary index updated at exactly these two points (set, remove), and it is that existing (Symbol, Symbol) index which makes invalidation tractable — when a category or item disappears, the cells naming it are found without a scan, so the records built from them are too.

It is deliberately not regenerated from the layout on demand. That would be O(visible cells) per resolve and would only ever cover the visible window, so a record scrolled out of view would look identical to a record that no longer exists — conflating "reject, nothing to write to" with "apply".

It is not serialized: Deserialize rebuilds it through set, and every version starts at zero, which is correct because a freshly loaded document is a new document_generation and no client holds a basis against it.

cells == 0 is a tombstone, not a reason to forget the entry. A record that is deleted and then re-created must not hand a client holding the old basis its own version number back; dropping the entry would reset the counter to zero and make a stale basis look current, which is the silent accept the basis check exists to prevent. The counter tracks liveness, not writes — set bumps cells only for a genuinely new cell, and version is moved by the observed diff in App::apply_effects, never from set.

resolve_record returning None means the record is gone — never stored, or every cell removed, which is what happens when a category or item it named is deleted. That is one arm of staleness and needs no version comparison: there is nothing to write to. record_version reporting 0 for an unknown surrogate is deliberate: a basis composed against an absent record and one composed against an untouched record are the same claim.

9. Categories

CategoryKind is Regular | VirtualIndex | VirtualDim | VirtualMeasure | Label. Business rules match on the enum rather than sniffing name prefixes.

CategoryKind::category_name() is the one place the name↔kind mapping is written down, so adding a variant forces a decision there (improvise-2lh). of_name(name) is safe to use in place of looking the Category up, because Model::add_category hands back the existing virtual category rather than shadowing a reserved name — stored_virtual_categories_match_their_names is what makes that sound. Label categories are user-named and so read as Regular from of_name; consult the stored Category::kind when the distinction matters.

VirtualMeasure items come from two sources: numeric data fields listed in the file, and formula targets added implicitly. They are virtual because formula-derived items are implied by the formula definitions — listing them explicitly would be redundant in the file format and confusing in the UI. add_formula does not add items to _Measure; use measure_item_names() / effective_item_names("_Measure").

Label is a high-cardinality per-row field (description, id, note): stored alongside the data so it shows up in record/drill views, but defaulting to Axis::None and excluded from the MAX_CATEGORIES = 12 pivot limit and from auto Row/Column assignment.

Category::add_item deduplicates by name and assigns IDs from a private counter, so external code cannot fabricate an ItemId.

Seating takes the kind, because a name cannot carry it (improvise-cj4u)

View::on_category_added takes a CategoryKind beside the name. It used to take the name alone and ask of_name, which reads Label as Regular by design — so the default view got a label on Axis::None (because Workbook::add_label_category set it explicitly afterwards) while Workbook::create_view, which only ever had names to hand, seated the same label on Row. A high-cardinality description column therefore became a pivot axis in every view created after it, and on an already-pivoted document it landed on Page, filtering the whole document down to one description.

Two things follow, and the second is the one worth carrying.

The parameter is the fix, not a set_axis after the fact. The old add_label_category seated the category and then corrected it, which is two answers to "where does this category go" — and only one of the two callers of on_category_added knew to write the second. The correction is now deleted: seating is told the kind and answers once. add_label_category_sets_none_axis_ on_all_views and label_category_defaults_to_none_axis both still pass, and both fail if the parameter is ignored, so the consolidation kept its evidence.

of_name survives, narrowed. Every remaining caller either holds no Category (the view's own is_records_layout, holds_a_regular) or is test code building a view from bare names. That residue is real: holds_a_regular asks of the categories already seated on an axis whether one of them pivots, and it asks by name, so a label a user deliberately moved onto Row still counts as occupying it. That is the right answer — a category the user put on Row is pivoting there — but it is an answer reached by a different route from the one above, and a future View that stores kinds should collapse the two.

Removing a name removes the formulas targeting it (improvise-g3no)

Model::remove_category has always ended by dropping every formula whose target_category is the category — that is why Dirty::of(LayoutGeometry) reports formulas: true (gotcha 35). remove_item did not do the same one level down, and the asymmetry was a live bug rather than an omission of symmetry for its own sake.

_Measure's items are explicit items ∪ formula targets, so a Profit that exists only because a Formula does is not in the category's own item list. remove_item called Category::remove_item, which no-opped, then swept the cells naming it, of which there are none because the value is derived. effective_item_names re-synthesised the row from the surviving formula and it came straight back — while both front ends reported success, each having composed an optimistic past-tense status beside the mutation.

Two things about the shape of the answer. The lie was a symptom, not the defect. The obvious reading is gotcha 20's ordering rule violated — the fallible effect first, the optimistic status second — and the obvious repair is to give remove_item an answer the front ends let win. That repair is unavailable on the browser side, in two independent ways: SetStatus is ViewOnly and never crosses the wire, so a host-side refusal cannot reach a client; and a client cannot decide for itself, because nothing in the projection stream says which _Measure rows are derived — FormulaCache is Vec<String> of Formula::raw, and recovering a target from that is a second home for the unquoting §34 says has exactly one. Restoring the sweep makes the status both ends already compose true, with no wire change and no second classification.

What this does not answer, and is declined rather than forgotten. doctrine(a_status_line_composed_beside_a_mutation_lies_when_the_mutation_no_ops) generalises to "a mutation whose failure is reportable must RETURN whether it acted", and remove_item still returns unit. Restoring the sweep removes the reachable lie — from either panel the item name comes off the tree, so it exists — and leaves a narrow one: a wire ModelCommand::RemoveItem naming an item the document does not have is still answered with a past-tense success. Closing that means threading an answer through Workbook::remove_item and effect::RemoveItem, and it is improvise-nwgk rather than done here.

And the sweep is not restricted to _Measure. A non-measure target is a real item (add_formula creates it), so removing that item used to leave a formula aimed at nothing. The predicate is target_category and target: deleting Cost must not delete Profit = Revenue - Cost, which references it but does not target it.

A required virtual category cannot be removed (improvise-3b2b)

The category panel deliberately lists _Index/_Dim/_Measure (improvise-vjms — a decision about visibility), and its delete key treated them as ordinary removable categories. d at the panel's default cursor 0 therefore deleted _Index from a fresh session, and GridLayout's records-mode detection keys off _Index on Row, so records mode silently stopped working while the status line said it was on.

Model::remove_category refuses when CategoryKind::of_name(name).is_virtual(). The guard is at the model rather than in the two Cmds because hiding a key leaves ModelCommand::RemoveCategory unguarded on the wire. It reads of_name, the one home of the mapping, so a Label category — user-named, and Regular to of_name — stays removable.

10. Workbook exists to break the Model ↔ View cycle

Model knows nothing about views; View depends on Model, one direction. Cross-slice operations — adding or removing a category must update both the model's categories and every view's axis map — live on Workbook, so Model stays pure data and improvise-core can be extracted without pulling view code along. That is also why AddCategory/RemoveCategory are LayoutGeometry effects rather than CellData ones (gotcha 3).

Workbook::new seeds a Default view, registers the three virtual categories through on_category_added (so all three land on Axis::None), then bumps _Measure to Axis::Page so aggregated pivot views show a single measure at a time (improvise-kos, commit 709f2df). Leaving _Index/_Dim on None keeps pivot mode the default — records mode activates only when the user moves both onto axes.

Workbook::add_item errors on an unknown category so callers do not reach through model.category_mut(..) and invent their own handling (improvise-tvt). There is no Model::add_item.

11. Virtual views

_Records and _Drill (view::VIRTUAL_VIEW_NAMES) are scratch views the app builds on demand. Both put the virtual _Index/_Dim categories on axes, and those categories are never persisted — so these views are rebuilt from scratch each time rather than saved (improvise-60z).

What the view must be is model-side; how a front end gets there is not

view::layout answers what a virtual view is, as data: records_spine() (_Index on Row, _Dim on Column — the pair GridLayout auto-detects records mode by), records_steps(model) and drill_steps(key, none_cats), each a Vec<ViewStep> of Axis and PageSelection assignments. Nothing in them names an Effect, an App or a front end.

The command layer answers how: cmd/grid.rs::steps_into_active_view maps each step to the effect::SetAxis/SetPageSelection a TUI keypress applies to ViewTarget::Active. That is the only thing the two rebuilders still have in common, and it is three lines of translation.

The split exists because a second front end has to reach the same view without the effect layer — a browser composes ModelCommands naming _Drill, not effects targeting an active view it does not own. Design-principles §5 is the rule: a derivation done twice is the defect even when both copies are right today, and before this split the drill's filter placement and the records spine were spelled out twice in cmd/grid.rs with nothing keeping them in agreement. drill_constraint and drill_snapshot were already model-side for the same reason and are untouched; §8 says why their order is the thing that cannot be derived a second time.

What deliberately did not move is DrillState. It holds an Rc and its pending_edits are uncommitted writes belonging to whoever staged them — the session-local residue of gotcha 30 — so it stays on App, which is where that tag puts it. Separating the view configuration does not make the staging map portable, and a decomposition that claimed otherwise would be the interesting half of this ticket answered by assertion.

create_view replaces, and unused_view_name is why it may keep doing so

Workbook::create_view inserts over whatever holds the name. That is not an oversight: "rebuilt from scratch each time" is that replacement, and both rebuilders reach it through effect::CreateView — DrillIntoCell and the records-mode command each emit a CreateView of their fixed name on every entry, so an entry into records mode would otherwise stack views or refuse. The third production caller, parse.rs::build_view, guards with contains_key and so cannot observe the difference.

So the two callers that need a new name are the user-facing ones, and they are the ones that must not name a view that exists. Before improvise-vw47 both spelled format!("View {}", views.len() + 1), which is the count and not the name: delete View 2 from a workbook holding Default, View 2, View 3 and the count says View 3, which is taken. create_view then did exactly what its other callers need and replaced it — the user's configured view silently reset to defaults, and no view added. A symptom that looks like nothing happened.

Two fixes were rejected in favour of the helper, and both are worth naming because each turns a quiet wrong answer into a loud one at a caller that was never wrong. Making create_view refuse a name it holds breaks records mode outright — the key stops working, which reads as a better error and is a worse bug. Making it rename silently gives the rebuilders a _Records 2 that RECORDS_VIEW does not name and nothing switches to.

view::unused_view_name therefore answers only the question the two user-facing resolvers ask, and create_view keeps its semantics untouched. It takes an iterator of names rather than a &Workbook because the browser resolver holds no workbook — it has ViewListCache, a Vec<ViewSummary> that project::view_summaries maps one-for-one off workbook.views, so the two front ends see the same list and derive the same name by calling the same function, which is what design-principles §5 asks for and what tests/lockstep.rs witnesses. It starts its search at the count rather than at 1 so that the common no-gap case keeps its existing answer; starting at 1 would rename the first view a fresh workbook creates from View 2 to View 1.

12. axis_of panics, try_axis_of does not

Registration is guaranteed for every model category: Workbook::new, add_category, add_label_category and create_view all call View::on_category_added for every view. So any name obtained from Model::categories or from the view's own category_axes is safe for axis_of. Use try_axis_of when the name comes from anywhere else — cycle_axis does, and no-ops on an unknown category to match set_axis (improvise-t8s).

record_cell_key is a match, and the match is the guard (improvise-ir7l)

GridLayout::record_cell_key(row, col) is the one mapping from a records position to a model cell, and it answers Some for RecordCol::Measure alone. The other three arms are spelled out rather than swept into a _, and that exhaustive match is the whole guard: a new RecordCol variant fails the build until someone decides whether it names a stored cell.

There is no test for the Category arm, and the reason is not that the wrong answer would be harmless. It would be a real fault, and a subtle one worth naming so nobody re-derives it wrongly. RecordCol::Category(name) carries a category name, while RecordRow::cell_key(arg) puts its argument under _Measure. So answering Some there yields coords + _Measure=<category name>, and Model::formula_for would match it whenever a _Measure item happens to share a name with a regular category — projecting CellId::Derived for a coordinate column, which is never a formula target and which the pivot path does not have at all. The two front ends would then disagree, which is the design-principles §5 defect this ticket exists to close. RecordCol exists for exactly that collision: its doc says the enum resolves a column from its index "so a category and a measure sharing a name stay distinguishable".

What earns the decision to leave it untested is narrower: the reachable version of that fault is a new variant answered carelessly, and the exhaustive match makes that unrepresentable rather than merely unlikely (doctrine(an_invariant_is_made_unrepresentable_not_validated)). A test would pin behaviour the type system already pins, and would not survive the arm being renamed. Found in review, and the first recorded reason for declining it was wrong — it claimed the injected behaviour was defensible by analogy with a formula targeting a regular category, which is a different key entirely.

13. Route then aggregate: SUM(<expr>) folds over the member records

improvise-4zl7, and the one place the evaluator reaches past the cell it was asked about.

The problem. A formula is evaluated at a CellKey. In an aggregating cell evaluate_aggregated aggregates each Expr::Ref first and evaluates the expression once on the totals. That is exactly right for a linear formula — SUM(Revenue) - SUM(Cost) and SUM(Revenue - Cost) are the same number — and exactly wrong for anything that routes. IF(Cost < 50, 1, 10) over records costing 10 and 100 answers 1 + 10 = 11 per record and 10 on the total, because aggregate-then-route is not route-then-aggregate. Every fitted tree (improvise-8xhy, improvise-vx3) is such a formula, so it meant one thing on a record row and something else one row up. The user's ruling was to aggregate the rule over the members.

The mechanism, and why it needed no new vocabulary. SUM(e) already means "fold e over the members". It simply did not: eval_agg specialised Expr::Ref and, for every other inner expression, summed the raw cells matching the partial key and ignored the expression entirely. It now folds — one entry per member record, the expression evaluated at that record's own coordinates:

partial   = context.without(target_category)    (plus any inline WHERE)
records   = distinct(matching_cells(partial).map(key.without("_Measure")))
values    = records.map(|r| eval(inner, context.with(every coord of r)))

Three consequences worth stating.

The rule is written once, and so is the evaluator it folds with. eval_agg is shared by eval_expr and eval_expr_cached, and both hand it the same eval_at: the record-level eval_expr. That is not a convenience, it is the correctness. A member key is a record, and formula_cache is keyed by stems with none_cats stripped — so the cached evaluator's Expr::Ref can never hit the cache at a member key, falls through to aggregate_raw, finds no raw cell behind a formula target, and answers Err("ref:…"). A first version did exactly that, and the two evaluators then disagreed on any routing formula reading a derived measure: Number(4.0) uncached, Error("ref:Profit") cached, which is the very divergence this section exists to remove (§3: "a rule stated in one and not the other is a live bug").

And making the lookup "work" would have been worse than the error. The only way to hit the cache from a member key is to strip none_cats off it — which returns the group's value for that formula and hands it to every member. That is aggregate-then-route again, laundered through a cache lookup, and it would have been silent. The cache cannot answer a per-record question, so the fold does not ask it. Only Expr::Is needed an arm in each evaluator, and since the fold routes through eval_expr, the cached arm is now reached only by a bare Is outside a SUM — which is what the_cached_evaluator_has_its_own_working_coordinate_predicate pins.

The Expr::Ref fast path stays, and it is the same fold specialised: a record's Ref is that record's own cell, so summing them is summing the matching values. Two of its premises were unstated and both were false (improvise-s08t, improvise-s2w3), which is worth recording as a shape: a specialisation defended by an argument is only as good as the conditions the argument assumed, and neither condition was being checked.

The name must resolve. The narrowing was written if let Expr::Ref(name) = inner && let Some(cat) = find_item_category(name), so a name that resolved to nothing left the partial key unconstrained — SUM(Typo) aggregated every numeric measure matching the context and answered a plausible total. A missing identifier now fails before the fast path is reached, with the same ref:{name} the record-level evaluator uses.

The name must be stored. A formula target is a _Measure item that nothing wrote (improvise-nrs), so there are no raw cells behind it and the raw scan returns nothing: SUM(Double) answered 0 where SUM(Double + 0) folded and answered 20. The premise of the specialisation — "a record's Ref is that record's own cell" — is exactly what fails there, so the answer is to ask whether it holds rather than to delete the path. formula_for is the one home of "is this cell derived", so the check is that call and not a second derivation; a derived ref falls into the fold, where eval_expr's own Ref arm routes through evaluate_depth and sees the formula. A target that also has stored cells reads as the formula, because formula_for wins there too.

It differs in exactly one place — a record with no such cell is skipped there and an error in the fold. That is deliberate: a bare SUM(Revenue) asks for the values that exist, while SUM(<expr>) asks the expression of each member, and an expression that cannot be evaluated is an error rather than a zero. Silently dropping members is how a routed sum would quietly answer for a subset.

The asymmetry's blast radius is wider than it sounds, and worth stating: gotcha 7 makes a pipe-quoted text cell a first-class .improv value, so one text cell anywhere in a member set turns the whole aggregated fitted cell into Error("ref:Cost"), where the bare-ref path would have dropped it and returned a number. That is the loud failure this repo prefers to a quietly wrong sum, but it is a real behaviour difference and not a detail.

Records come out in DataStore insertion order, deduplicated by an IndexSet, for §8's reason — hash order here would make a fitted cell drift in its last bits between processes.

Interaction with the fixed point (§3) and formula_cache. None of this is new state. The fold reads raw data through the same evaluate_depth / aggregate_raw the non-folding path uses, so a formula cell still depends only on raw cells and on other formula cells' cached values; recompute_formulas iterates the same stems to the same fixed point, and formula_cache still holds exactly the aggregated formula cells it held before. The fold does not cache and does not memoise: it is O(members) per evaluation per pass.

Measured, because the cost was a blocker and not a footnote (improvise-9iyv; crates/improvise-core/examples/fold_bench.rs, five runs per point, best–worst). At 18,050 cells with one routing formula and Tag on Axis::None, recompute_formulas takes 18.1–19.2 ms release and 132.9–135.1 ms debug, against 7.1–9.0 ms / 71.7–74.6 ms for a linear Rev - Cost. That is 2.3× a linear formula and linear in cells: 400 → 0.4 ms, 2,000 → 2.0 ms, 18,050 → 18.1 ms, so 45× the cells costs 45× the time.

It was not always linear. Routing the fold through eval_expr_cached — the first version, above — made every member's Ref miss the cache and fall into aggregate_raw, one poorly-narrowed scan per member: 51.1–53.3 ms at the same 18,050 cells and superlinear with it (45× the cells cost 85× the time). So the divergence and the stall were one defect, and fixing the first fixed the second.

And "18,050 cells" was not a sufficient description of that workload. At the same cell count the old code cost 51 ms at a 25×361 shape and 244 ms at 95×95 — a 4.8× spread from group count alone, which is what a per-member scan predicts and a constant factor does not. After the fix the two shapes are 16.9–17.2 ms and 16.4–17.5 ms: indistinguishable. The shape-dependence being gone is the confirmation of the cause, more than the absolute number is. Re-measuring this means stating the shape, not just the cells.

The fixed point runs exactly 2 passes (measured, not assumed), so hoisting the member set out of the loop has a 2× ceiling on the fold's share and was declined: a memo is state, and a memo keyed by anything less than the whole partial is a second answer to "what does this cell depend on".

One asymmetry not to tidy: the cached path hands eval_expr a fresh MAX_EVAL_DEPTH at each Agg, while the uncached Agg threads its own depth through. The second is what bounds a chain of folded formulas; making them agree by resetting both would unbound it.

Expr::Is(Filter) — a coordinate predicate as a number. Cat IS Item is 1.0 when the cell's own Cat coordinate is Item and 0.0 otherwise. It keeps the evaluator f64-only — no string comparison enters eval_expr — and it reuses the primitive Formula::filter already had, context.get(&cat), including its unwrap_or(false): a cell with no Cat coordinate is not that item. Under route-then-aggregate that arm is reached only where a record genuinely lacks the coordinate, because the fold hands the expression a record key. Evaluated bare, outside a SUM, it degrades exactly as Formula::filter does — which is why a fitted tree is always emitted wrapped.

14. A collapsed group is one row, and its cell folds the group (improvise-6qet)

The ruling. "When collapsing a group, I'd generally expect the cursor to move to whichever place the group collapsed into" (user, 2026-09-23). The shape chosen from a presented fork: a collapsed group contributes one data row, whose cell is an aggregate over the group. Before this, expand_category emitted the group's GroupHeader and then continued past every item, so a collapsed group had no data row at all. row_group_for is indexed by data row, so nothing on the grid could name the group once it was collapsed: z pressed twice collapsed the neighbour, and a collapsed group could be reopened only by :toggle-group.

The fix is to emit one DataItem where the continue was, and every cursor-addressed question then answers itself by arithmetic. With Q1={Jan,Feb}, Q2={Apr,May} and the cursor on Apr (row 2), collapsing Q2 gives rows [Jan, Feb, Q2], so row 2 is Q2 and a second z names Q2 again. z z is an involution without anything being taught about involution, and neither ViewCursor nor row_group_for changed.

The row

  • Its tuple is the group name (Month=Q1). AxisEntry::DataItem became a struct variant { tuple, collapsed } rather than growing a second variant, because a new variant would be silently missed by matches!(e, DataItem(_)) sites; changing the shape made the compiler list all six external matches.
  • collapsed cannot be derived from the tuple: a group and an item may legally share a name. A parallel Vec<bool> was rejected for the reason gotcha 1 gives prune_axis_items a predicate — pruning shifts indices.
  • The flag is inherited down cross_product, so a category nested under a collapsed group (Q1/North) is also a fold: no record carries Month=Q1 whichever inner coordinate sits beside it.
  • A group's items need not be contiguous (A[G1], B[G2], C[G1] is legal), so expand_category keeps a folded list: a collapsed non-contiguous group folds once, and its header is not re-emitted on the repeat — that header would open a run with no row in it.

The cell

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 filled only by .improv > lines. Asking it answers "not a group" for every document built in code, and the symptom is the fold's row pruned as empty.

  • 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. 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). The memo is now shared across one grid build (improvise-h6z1). viewport::grid_body makes one FoldMemo and passes it through GridLayout::evaluate_in and column_total for every cell and total, so a fold row's chain is evaluated 9 times, not 45. A memo is valid only for the model state it read. That is why it is made per build and never stored on GridLayout, which outlives cell writes. The memo is keyed by the fold key and the span's categories. Within one grid build the span half cannot yet differ for one key, because a cell's key always names the row categories and a total's never does. It is kept because evaluate_spanning_in is public, and a caller sharing one memo across two spans would otherwise read the wrong fold. No test can see it today.
  • 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 column's total does not move on z, and a group whose every member is hidden has no row. That holds for every aggregator and for a formula column only since improvise-wm20, because the strip used to sum the drawn rows. That moved a Max measure's total on z, and a ratio's (PR #203's review measured Margin 1.90 expanded and 1.30 collapsed). A total is now the column's cell as if every drawn row were one fold, so collapsing a group changes nothing it reads. The fold's machinery is shared through Model::evaluate_spanning: its Span adds, to the groups a key names, the categories a key omits, which it expands to their covered items. 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

The row is not a record, so project.rs::cell_id answers CellId::Aggregate when GridLayout::folds_a_group(row, col) — the one home of that question — whatever is on Axis::None. Without it, a document with nothing regular on None projected the fold as CellId::Record, offered it for editing, and a SetCell would have written the coordinate Month=Q1, which is not an item. render.rs::editable is untouched (ADR 002: the variant decides). EditOrDrill asks the same predicate and declines out loud: drilling would filter on Month=Q1, which no record carries, and show an empty list. Guarding that one command left >, p, H and an Enter advanced onto the fold all still writing or drilling there (improvise-gw18, yk2s). Every command that takes the cursor's cell now goes through a door that asks this predicate, or asks cell_id, which does: context/code/command.md, "The cursor's cell has three doors".

The cursor, and why the order is the fix

ToggleGroup is Model(LayoutGeometry), and apply_effects resets the cursor when it observes the frame move inside the loop (gotcha 24). So ToggleGroupAtCursor emits ToggleGroup first and the cursor placement after; the swap is the rival, and it lands every toggle at (0, 0). The placement also restores the scroll offsets the reset zeroed.

Where the cursor lands is asked of the layout the toggle will produce (GridLayout::row_run_start(cat, group, prefix) on a view with the toggle applied), not of the current one. The same answer serves collapse and expand — the group's first row — and it matters when the group is at an inner level: collapsing Month under Region shrinks every earlier Region's run too, so South/Apr at row 6 lands on South/Q2 at row 5, not row 6. That is the only test the "ask the current layout" rival fails.

What this does not do

  • Part 2 of the ruling (a toggle carrying the group it acted on, so a later command in the same batch can be invalidated) is not added: nothing binds z as a Binding::Sequence, and if something did, both toggles would resolve against one pre-batch context and name the same group, so the pair is a no-op — which is what z z means. A mechanism with no reachable case is not built.
  • CSV export writes the fold as a row, because an export writes what the grid draws (improvise-iot6); a collapsed group is its own total row there too.
  • Under prune_empty an empty collapsed group now disappears header and all, where improvise-lhi4 kept its header. It is a narrowing, and deliberate: the header was kept because it was the only way back to the rows, and the fold row is now that way back — an empty fold has nothing to show.
  • The browser still cannot press z (toggle-group-under-cursor resolves Upstream), so the cursor half takes effect in the TUI only.

PageAxis — what constrains, and what the user may turn (improvise-p1ym)

GridLayout::page_coords answers two questions that gotcha 27 says must not be confused. The bar at the top of a view names every category constraining the visible records, which is View::filtering_cats() — Page ∪ Filter — because a drill fixes its coordinates on Filter and the user should still see them. [/] cycle only what the user may act on, which is categories_on(Axis::Page).

Those were two walks before: GridLayout did the first, navigation.rs:: page_cat_data did the second, and each spelled the Page-vs-Filter rule itself. Two spellings of one rule is the shape design-principles §5 calls the disease, and here it had already leaked into the protocol — GridCache::page_coords was a verbatim copy of the layout's list, so a browser handed it would have tried to page a Filter axis.

So page_coords is now Vec<PageAxis>, and items is Some exactly when the axis is navigable. The rule lives in the type: a Filter axis carries no item list, so no caller can compose a cycle for one, and PageAxis::cycle() is the one place that pairs a list with the index its selection sits at. page_cat_data and the browser's page_cycle both read it rather than deriving it.

Two consequences worth stating. The list is Model::effective_item_names, the same call that already chose the default selection, so the selection and the list it indexes into come from one place by construction — page_cat_data used ordered_item_names before, which for a _Measure page axis meant the default selection could be a formula target [/] could not reach. That incoherence is gone; the cost is that [/] over a _Measure page axis now visits formula targets, which is what the column list shows anyway.

And the list is not filtered to visible items. Hidden items are still cycled, because that is what the TUI has always done, and making the browser differ would be the divergence rather than the fix. Changing it is a change to both, together.

The number format — format.rs (improvise-711n)

A view's number_format is a printf-style picture, and only that. A , anywhere turns on thousands grouping; a . introduces the precision, which is the leading digit run after the last .; anything following those digits — a conversion letter — is ignored. So ,.0, ,.2, .2, ,.2f and %.2f all name what they look like they name.

Why the trailing f is accepted rather than refused. It round-trips into documents today and always has. improv.pest spells the line format_line = { "format: " ~ rest_of_line ~ NEWLINE ~ blank_lines } — the grammar takes the format string as an opaque line, so format: ,.2f is a document the grammar accepts; :set-number-format stores whatever the user typed verbatim (cmd/effect_cmds.rs), and format_md writes view.number_format back out unchanged. Refusing the spelling at load would therefore reject documents this repo's own grammar admits and its own .improv example spells, to no benefit: by gotcha 8 the format is display-only, so misreading it costs a wrong pixel and can never cost a wrong number. A refusal here buys nothing and costs a document.

Why printf and not Excel. The two conventions disagree on exactly the spelling that distinguishes them: .00 is precision zero to printf and two places to Excel, so a parser that tried to honour both would have to guess. One is picked, and it is the one every spelling in this repository already uses. Consequences worth stating rather than discovering: 0.00 reads as (false, 0), and an Excel picture such as #,##0.00 reads as (true, 0) — grouped, no decimals.

Nothing is validated, deliberately. A spelling the rule cannot read names no precision and formats with none; there is no error channel and no Result, because the reader is display-only and four call sites would have to thread one.

What the bug was, and why six tests sat on top of it. The precision was read as fmt[i+1..].parse::<u8>() over the whole tail, so "2f" failed to parse and fell through to unwrap_or(0) — ,.2f displayed zero decimals, in the TUI, in the browser projection and in CSV export. ",.2f" and its ,.3f/,.4f siblings are what this workspace's own fixtures reach for, in about ten tests, and none could see it: they assert the stored string round-trips or use it as an opaque value to dirty a widget, or — in export_csv_quotes_commas_per_rfc4180 — they assert the rendered value starts_with("1,200"), which 1,200 and 1,200.00 both satisfy. A prefix check over a value whose suffix is the thing in question answers a neighbouring question, which is why that one is now an exact equality.

Only two of the three consumers are guarded. Reverting the fix fails format.rs's own unit tests and export_csv_quotes_commas_per_rfc4180, and nothing else in the workspace: ui/project.rs::grid_of (the browser projection) and ui/app.rs both call parse_number_format and neither has a test that renders a cell and asserts its digits. That gap is named rather than closed — those files are improvise-command, which was another wave's ground when this landed — and it is the more useful half of this entry, because it says where the next regression would go unseen.

That gap is now closed, and the sentence above is kept as the record of it. ui::project::cell_identity::a_formulas_format_reaches_its_own_column_and_no_other renders cells and the totals strip and asserts their digits (improvise-e0ex); ui/app.rs's own call is still unguarded, and now reaches the same FormatPlan rather than a second parse_number_format, so there is one fewer place to drift.

The decimal default and a formula's own format (improvise-e0ex)

View::new seeds ,.2, not ,.0. The symptom that forced it was Margin = Profit / Revenue drawing as 1: a 0.66 ratio under zero decimals, rounded half-away-from-zero by format_f64. Three things worth stating, because each was a candidate answer that is wrong.

It is a formatting bug and not an arithmetic one. Exact decimal arithmetic (ADR 005) would not change the rendering by one character — 0.66 under ,.0 is still 1 — so nothing here waits on it and nothing here touches the evaluator. Gotcha 8 is unchanged: format_f64 is called in rendering and nowhere else, and full f64 is what eval sees.

The cost is paid on whole numbers, knowingly. Revenue now draws 18,400.00. That is what an accounting application does, and it is the cheaper error: a currency column with two redundant zeroes is legible, where a ratio with none is wrong. A format cannot be guessed from a value — 0.66 is a ratio and 0.66 is also two-thirds of a dollar — so the default must be the one whose failure mode is noise rather than a false number.

The override is spelled in the formula, not per _Measure item. The user's ruling ("unless the format is explicit in the formula") picks the narrower of two nearly-coincident units: a formula target is a _Measure item (gotcha 7), so per-formula and per-measure differ only for a measure no formula owns — and such a measure is raw data, whose format is the view's job. So a formula may carry its own format, which wins for its target; a raw measure keeps the view's.

Where the format is resolved: FormatPlan

FormatPlan::of(model, view) is the one answer, and every render path takes it where it used to take a (comma, decimals) pair: GridLayout::display_text, ui::viewport::grid_body/compute_col_widths, ui::project::grid_in and persistence/export.rs::render_csv. Replacing the pair rather than adding a second argument beside it is the row_levels-next-to-row_labels rule from design-principles §5 — two answers to "what format is this cell" is the disease.

It is asked of the cell's CellKey, never of the axes. A cell aggregating over _Measure carries no target coordinate and so keeps the view's format, which falls out of the key rather than needing a rule. Deriving it from "is _Measure on a visible axis" would be a second derivation that could agree with itself while disagreeing with the cells.

The empty plan short-circuits, so a document with no formula format pays one is_empty per cell and nothing else; the lookup is otherwise a scan of the formula list, which is short and must stay ordered for the reason below.

The owner is the formula Model::formula_for names, and nothing else. FormatPlan holds every formula's (target_category, target, Option<format>) in the model's own order and takes the first coordinate match, which is the same rule and the same order evaluation uses to decide whose value the cell is. Two things that shape enforces, both of which a HashMap keyed by target got wrong and review caught (Codex, PR #125):

  • A key may be claimed by two formulas, because a target category need not be _Measure — - Tax = Revenue * 0.1 [CustomCat] is in the format's own documentation. Iterating a HashMap answered from whichever category hashed first, so the displayed format was nondeterministic per process while the value was not. That is gotcha 18 exactly: an index whose values are hashed is for lookup, never for choosing between candidates.
  • An owner that names no format still owns the cell. Indexing only the formatted formulas let a later formula's FORMAT reach a cell whose value it does not produce. So the list holds every formula and a match with format: None falls through to the view — it is a match, not a miss.

The list is dropped entirely when no formula names a format, so the common document pays one is_empty per cell rather than a scan.

A column total is resolved from the column, never from a row. GridLayout::column_key(col) is the page/filter constraints plus the column's own tuple — the coordinates that survive the row aggregation — and GridBody carries the resulting col_formats so the strip and the cells cannot disagree. Reading it off row 0 was the first version and is wrong twice over, also found by review: a formula may target a row-axis item (its target category is any category), so the total inherited a row-local override for a row it had summed away, and re-ordering the rows changed the total's spelling. The guard is a_column_total_does_not_inherit_a_row_local_format, which builds the same grid with the owned row first and last and requires the two strips to agree.

The grammar: FORMAT <spec> in the formula, not in the .improv line

formula.pest gains format_clause = ${ ^"FORMAT" ~ sp+ ~ format_spec } after the optional WHERE. improv.pest is unchanged, and that is the design rather than an omission: formula_raw already takes the ## Formulas line verbatim up to the optional [Category] suffix, so a format inside the formula text is carried by the existing grammar with nothing to add. Two consequences. The grammar-walking generator (which reads improv.pest, not formula.pest) is untouched, so there is no new spelling for it to violate. And the same text a user types into the TUI formula minibuffer is the text the file carries — one namespace, one spelling rule, exactly as gotcha 34 says of names.

sp+ rather than a !bare_char lookahead, for WHERE's reason (gotcha 34): a predicate is invisible to a generator. FORMAT joins quote::KEYWORDS, so an item literally named FORMAT is written |FORMAT| — the keyword guard is canonical and this is one more entry in it, not a new rule.

format_spec is (ASCII_ALPHANUMERIC | "," | "." | "%")+ — a bounded token where the view's format: line is rest_of_line. The line can afford to be opaque because a newline ends it; a formula clause cannot, because something may follow. The set covers every spelling parse_number_format can read (,.2f, .4, %.2f) and nothing the tokenizer would have to disambiguate.

The trap this feature sets, and what is asserted instead

format_md writes Formula::raw, so a format inside the raw text round-trips whether or not the walker parsed it into a field. That is gotcha 34's bug exactly — a target that kept its pipes survived every round-trip test because raw was unaffected. So the round-trip test asserts Formula::format, the parsed field, and its raw assertion is there to show that raw cannot be the witness. Injecting the fault (a walker that drops the clause) fails that one test and nothing else in improvise-io — 242 other tests, including the whole format_md↔parse_md characterization suite and the grammar-walking properties, pass. That is the measurement, not an argument.

What persistence does and does not do here

The user's constraint — "should not affect persistence at all, except to store format metadata" — holds by construction and the job was to not break it. format_f64 is display; persistence/format.rs::format_number is a different function in a different crate writing Rust's shortest round-trip form. The regression to watch for is routing a save through the display formatter so the file matches what the user sees: a model has many views (gotcha 29), so there is no single right answer and the last save would win. Gotcha 8 forbids feeding a rounded value back into eval; this is its persistence twin.

What did change on disk is metadata only: newly created views write format: ,.2 where they wrote format: ,.0, and a formula line may now carry a FORMAT suffix. Existing documents are unaffected — format_md has always written the format: line explicitly whenever number_format is non-empty and View::new's value has never been empty, so every document improvise has ever written carries its own and no migration exists to get wrong.

One consumer that is display but reads like persistence: CSV export goes through display_text and therefore through the view's format, as it always has. A document with no explicit format: line now exports two decimals. That is pre-existing behaviour meeting a new default, not a new coupling.

16. A non-finite cell value has one spelling, and the format chose it (improvise-55v2)

serde_json writes a non-finite f64 as null, and null does not read back as one. So CellValue::Number(f64::INFINITY) crossed the wire as {"Number":null} and the frame carrying it was unreadable: the websocket reader skips an unparseable frame and the worker logs a console error, so one poisoned cell silently dropped a whole batch. to_string returned Ok, so the send site could not notice either.

The ticket offered two answers — an explicit representation, or a refusal before sending — and the .improv format had already chosen. It carries non-finite numbers as first-class values: format_number writes inf, -inf and nan, the grammar's bare_value accepts them, and the walker maps them back. All three round-trip losslessly, measured before anything was changed. A refusal would therefore make a legal saved document unprojectable to a browser, which is a new failure rather than a fix. It also settles the second location the ticket named: resolve.rs's typed() accepting inf is correct, not half a defect.

So Number serializes through an untagged adaptor — a JSON number when finite, one of the three tokens when not. A finite cell's bytes are unchanged, which is the whole of the compatibility story, and a JSON integer still reads as a number rather than being taken for a token.

The tokens are one home: nonfinite_token / parse_nonfinite beside CellValue, called by the adaptor, by persistence/format.rs and by persistence/parse.rs. Three copies stood before and a fourth was about to be written, which is the shape gotcha 34 named for pipe quoting. format_f64 is deliberately not folded in: it is the display layer (gotcha 8) and may want a different glyph one day, where these three are a format.

What makes this checkable rather than asserted is that the guard compares the two carriers to each other — a_nonfinite_cell_is_spelled_the_same_on_disk_and_on_the_wire reads the token off the ## Data line and off serde_json and requires them equal. A second home that happens to agree is harmless; one that drifts fails there, and re-copying the tokens into format.rs with one letter changed was injected to prove it does.

17. An editor opens on the value, not on its Display (improvise-umi4)

CellValue's Display is a screen spelling: a non-whole number is written {:.4}, so 1.23456789 reads 1.2346. Every editor seed used to be that string (display_value_in's pivot arm through get_cell().to_string(), its records arm through records_display, and project::cell_display's records arm spelling it again), so pressing i and then Enter with nothing typed stored 1.2346. The screen looked the same before and after, which is what made it silent data loss rather than a visible bug. The browser seeds from the projected CellDisplay::edit_text, which came from the same call, so it had the same bug.

CellValue::edit_text is the one home of the seed. A number is written as the shortest decimal that parses back to the same f64, which is what Rust's f64 Display already guarantees (no precision argument, no exponent). The brief pointed at improvise-gp's number_text as the implementation to reuse. It is not a second implementation of this one: it takes an f32 and searches fixed precisions for the shortest f64 expansion of it, because a threshold must route the same under f32 and f64 evaluation. For an f64 the standard library's spelling is the same answer with no search. A non-finite number takes §16's token (inf, -inf, nan), which typed() also reads back, so the editor and the .improv format spell it one way.

Text and errors keep Display. Text is written unchanged, and an error cell has no edit text on the wire anyway. Display itself is untouched: the grid still rounds (gotcha 8), only the seed changed.

What this does not fix: a text cell whose text reads as a number. "007" seeds "007", and typed() classifies that as Number(7.0). The seed is exact, but the classifier has no way to know the cell was text. That is improvise-ixxx.

18. Which formula an edit means (improvise-km71)

A Formula has no stored id. It lives in Model::formulas: Vec<Formula> and is identified, in practice and by add_formula's upsert, by (target, target_category). FormulaId is that pair behind one opaque type: its fields are private, Formula::id() is its only constructor, and nothing outside improvise-formula can read a name out of it. That is deliberate. The stable-identity work (improvise-sboz, ADR 005) will make a formula's identity something other than its names, and every signature that already says FormulaId — Model::formula, Model::replace_formula, ModelCommand::ReplaceFormula, FormulaSlot::Existing, FormulaEntry — then changes by changing this one struct. A name pair spread across those signatures would have made the swap a sweep.

The index is not a candidate. A position is what formula_cursor is, and the editor must not recompute its target from the cursor at commit (improvise-ixxx/2wu3's lesson): another client deleting an earlier formula shifts every index after it, and the commit would land on a neighbour. It would also put a position on the wire, which every_model_command_field_is_position_free makes a compile error.

An edit is its own operation, never remove-then-add and never add_formula. add_formula upserts on (target, target_category): fed an edit that changes the target, it appends a second formula and leaves the old one; fed an edit onto another formula's target, it silently replaces that formula. Model::replace_formula(id, formula) writes into the slot id names, so the position (and with it _Measure's item order, since measure_item_names appends formula targets in formula order) is kept, and it refuses when the new target belongs to a different formula.

A target change is a rename of the same formula. The id travels with the edit, so Profit = … edited to Gross = … is the formula that was Profit, in the same place. That is the least surprising reading of "edit this row", and it is what ADR 005 proposes. What a rename cannot carry under name identity is everything else that names the old target: other formulas' references, a view's hidden list, a measure_agg entry. Those cannot follow without rewriting raw text the parser does not print back, so the edit says so out loud: ReplaceFormula reports which formulas still mention the old name (Formula::mentions, over the expression and its WHERE), and their cells show the ref error rather than a silent wrong number. Under a stable id those references are what sboz makes follow. This holds for _Measure targets only. Outside _Measure a target is a real item, and a rename is refused until sboz (improvise-kb1d, context/code/command.md). A rename also leaves stored cells under the old _Measure name in ## Data, invisible because the formula shadowed them.

Concurrency is checked twice. FormulaSlot::Existing { id, was } is captured when the editor opens: the id must still resolve, and the formula must still read was. Either failing means another client removed, renamed or rewrote it meanwhile, and the edit is refused with the batch stopped, so the editor stays open over the user's text (gotcha 21). was is not redundant with a stable id: a concurrent replace keeps the id and changes the text, which only was can see.