improvise/context/code/core.md

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.

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. 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.

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.

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.

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).

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).