# Test evidence — `improvise-core` What broke, what fault was injected, and why each test can fail. Keyed by exact test name (design-principles §7). A renamed test orphans its entry; `scripts/check-comment-runs.py --check-evidence` catches a heading naming a test that does not exist. Only tests whose narrative is longer than the code appear here. Most tests need none. --- ## `model/types.rs` ### model::types::formula_tests::removing_a_measure_row_that_is_only_a_formula_target_removes_the_formula improvise-g3no, demonstrated twice in a real browser and once in the TUI. `_Measure`'s items are "explicit items UNION formula targets" (gotcha 7), so a `Profit` that exists only because a `Formula` does is not in the category's own item list. `Model::remove_item` called `cat.remove_item`, which no-opped, then deleted cells naming it, of which there are none because the value is derived; `effective_item_names` then re-synthesised the row from the surviving formula and it came straight back. Both front ends meanwhile composed an unconditional past-tense status beside the mutation, so the user was told it had worked. **The diagnosis the bug report and the ticket both reached was one level too high.** They read it as a truthfulness defect — the status is emitted before the fallible effect, the opposite of gotcha 20's `CommitBuffer` ordering — and asked for a `Removed` answer on `Model::remove_item` that the front ends could let win. That is unimplementable on the browser side and would have been discovered late: `SetStatus` is `ViewOnly` and never crosses the wire, so a host-side refusal cannot reach a client at all; and a client cannot decide for itself, because nothing in the projection stream says which `_Measure` rows are derived — `FormulaCache::formulas` is `Vec` of `Formula::raw`, and recovering a target from it means a second home for the unquoting gotcha 34 says has exactly one. The actual defect is an **asymmetry in the model**: `remove_category` has always ended by dropping every formula targeting the category, and the item level did not. Restoring it makes the row genuinely go away, which makes the status both front ends already compose *true*, with no second classification and nothing new on the wire. Inject the fault by deleting the `retain`. The fixture builds its own model rather than using `revenue_cost_model()`, which carries an explicit `Profit` item — against that fixture the test passes before the fix, because there really is an item to remove. That is the whole distinction the bug turns on, so the precondition is asserted rather than assumed. ### model::types::formula_tests::removing_a_regular_item_removes_the_formula_targeting_it The same asymmetry one level up, and the reason the sweep is not restricted to `_Measure`. A non-measure target **is** a real item (`add_formula` creates it), so removing it left a formula aimed at nothing. A `_Measure`-only fix would have satisfied the ticket and left this live. ### model::types::formula_tests::removing_an_item_keeps_a_formula_that_only_references_it The direction the fix must not overreach in, and it passes before the fix as well as after — it is there to stop the next change, not to catch this one. `retain` on `f.target == item` alone, or on `target` without `target_category`, both look plausible and both would take formulas the user still wants: deleting `Cost` must not delete `Profit = Revenue - Cost`. ### model::types::model_tests::a_required_virtual_category_cannot_be_removed improvise-uej8's second half, which is improvise-3b2b's subject. The panel deliberately lists `_Index`/`_Dim`/`_Measure` (improvise-vjms — a decision about *visibility*), and `d` at the panel's **default cursor 0** deleted `_Index` outright. The invariant that the virtual categories always exist (gotcha 7) was reachable by one keypress from a fresh session, and `GridLayout`'s records-mode detection keys off `_Index` on Row, so the symptom is that records mode silently stops working. The guard is in `Model::remove_category` rather than in the two `Cmd`s, because hiding the delete key leaves the wire mutation (`ModelCommand::RemoveCategory`) unguarded — 3b2b asks for exactly this boundary. It reads `CategoryKind::of_name`, the one home of the reserved-name mapping (improvise-2lh), so a `Label` category stays removable. The second assertion is the one that earns its place: putting the category back is not enough, since `remove_category` sweeps cells and formulas *after* removing the key from the map. It asserts a stored `Revenue` still evaluates, so a guard that returned too late fails. The opposite direction is already held by `remove_category_deletes_category_and_cells` immediately below. ### a_chained_formula_still_converges_after_the_circular_marker The anti-clamp half of improvise-7iww. `recompute_formulas` now publishes `Error("circular")` for any cell still moving after `MAX_EVAL_DEPTH` passes, and a guard that refuses everything would satisfy the reproduction just as well as the fix does. This test runs a three-deep chain — `Cost`, `Profit`, `Margin` — that converges in two passes and asserts the *numbers*, so a marker that fired on a converging chain fails here. Inject the fault by marking every cell evaluated in the last pass rather than only the ones that changed. A second, free cross-check lives in `an_unknown_name_inside_an_aggregate_errors_for_every_function`: it asserts the error text `ref:Typo` *after* `recompute_formulas`, so a formula that errors identically on every pass must be seen to **converge** rather than be overwritten with `circular`. The two fixes check each other. One arm of the loop is worth naming because it looks reachable and is not. A pass that computes `None` where the cache holds `Some` counts as moving and stores nothing, so it would spin to the bound and now be marked circular. The only `None` `evaluate_formula_cell` can produce is a `WHERE` filter that does not match falling through to `aggregate_raw` over no data — and `aggregate_raw` reads `self.data`, never the cache, while the formulas do not change during a recompute. So for a given key that branch answers the same thing on every pass, and a `Some`-to-`None` transition cannot occur. The pre-existing code spun the same sixteen passes here and left a stale value; of the two, publishing `circular` is the safer, so the arm is documented rather than changed. ### a_derived_measure_with_stored_cells_does_not_leak_the_raw_value improvise-s2w3's stated edge case. The fix routes a bare `SUM(Double)` through the per-record fold when `Double` is a formula target, and the interesting case is a target that *also* has stored cells: `formula_for` is the one home of "is this cell derived", so the fold reads `20` (the formula) and not `999` (the raw cell). Fails if the fast path is reinstated for a derived name, or if the fold were built to prefer raw data over `formula_for`. ### an_aggregate_over_a_derived_measure_sees_the_formula_in_both_evaluators improvise-s2w3, widened past the reproduction. `eval_agg` specialised `Expr::Ref` into a raw `DataStore` lookup; a formula target has no raw cell behind it (improvise-nrs), so `SUM(Double)` summed nothing and answered `0` while `SUM(Double + 0)` folded per record and answered `20`. core.md §13 defends that fast path and the defence still holds — it is the fold specialised to a ref naming a *stored* cell. What failed is the premise, not the specialisation, so the fix asks `formula_for` rather than deleting the path. The test runs `SUM`/`AVG`/`MIN`/`MAX` and both evaluators (direct `evaluate`, and `evaluate_aggregated(OverHidden)` after `recompute_formulas`), because §3 says a rule stated in one evaluator and not the other is a live bug. `COUNT` is excluded deliberately: `COUNT(Double)` and `COUNT(Double + 0)` both count members and agreed even before the fix, so including it would have added a passing case that discriminates nothing. Inject the fault by hard-coding `derived = false` in `eval_agg`. ### an_unknown_name_inside_an_aggregate_errors_for_every_function improvise-s08t. `eval_agg` narrowed its partial key only `if let Some(cat) = find_item_category(name)`; when the lookup failed the key was left **unconstrained**, so `SUM(Typo)` aggregated every numeric measure matching the context and returned a plausible wrong total. A typo is silent and looks like a number. The test asserts the error *text* (`ref:Typo`) across all five aggregate functions and both evaluators, so a fix that errors with some other message — or that errors only where the raw scan happened to be empty — still fails. Inject the fault by restoring `unwrap_or("_Measure")` in place of the `ok_or_else`. ### both_evaluators_answer_every_comparison_the_same_way improvise-vyrx's *behavioural* half, and it is deliberately the smaller half. The ticket's defect is that `eval_bool_cached` wildcarded `BinOp` where `eval_bool` listed its arms, so a new variant would error at runtime in one evaluator and fail to compile in the other — an asymmetry no runtime test can see today, because both paths answer `Err("type")` for every arithmetic operator. **The guard is the compiler**: with the wildcard removed, adding a `BinOp::Mod` fails the build at four sites (`eval_expr`, `eval_bool`, `eval_expr_cached`, `eval_bool_cached`) where it previously failed at three, and that was verified by adding the variant and reading the errors. **A sweep for the shape found a second instance one level up**, in the same two functions: both `eval_bool` and `eval_bool_cached` also wildcarded the outer `Expr` match as `_ => Err("type")`, so a new `Expr` variant that ought to be usable as a condition — an `And`, say — would have become a silent type error in *both*. That is not vyrx's asymmetry (the two agreed), but it is vyrx's defect: a new variant answered wrongly with nothing to notice. Both now list the six non-comparison variants, and adding an `Expr::And` fails the build at four sites where it previously failed at two. Verified the same way. **What the sweep also settled is which wildcards to leave**, which is the more useful half. `eval_agg`'s own `match inner { Expr::Ref(..) => …, _ => false }` is deliberately *not* exhaustive: it falls through to the **general** path — the per-record fold — which is the right answer for any expression that is not a bare ref, including one that does not exist yet. The dangerous wildcard is the one that falls through to a **refusal** or to a **specialisation**; a wildcard that falls through to the general case is safe by construction. The same reading clears `CellValue::as_f64`'s `_ => None` and `CategoryKind::of_name`'s `_ => Regular`, whose inputs are a value that is not a number and a name that is not reserved. What this test adds is the neighbouring property the exhaustive arms do not give: that the six comparison arms *agree in value* between the two evaluators. It runs each operator against a true and a false right-hand side, so an arm mistyped in one path (`Lt => lv <= rv`) fails. Both directions matter — a single right-hand side is satisfied by an evaluator that answers a constant. ### re_adding_an_existing_category_at_the_limit_creates_nothing_and_succeeds improvise-f86q. `Model::add_category` checked `MAX_CATEGORIES` *before* `contains_key`, so at twelve regular categories re-adding an existing name was refused with "Maximum of 12 categories reached" — a false statement, since the category exists and nothing would be created. Per gotcha 21 `effect::AddCategory` sets `abort_effects` on refusal, so typing an existing name into the records-mode `+cat` column at the limit aborted the rest of the batch: the improvise-2zq failure mode by another route. The fix is a hoist. The test asserts three things, and the third is what stops it from being satisfied by deleting the limit: the returned id is the existing category's, the category count did not move, and `add_category("C12")` — a name that really would create something — is still refused. ### a_hidden_dimension_list_reduces_to_one_of_two_states improvise-5jm5. `evaluate_aggregated` took `none_cats: &[String]` and consulted it once, for emptiness — so two lists with different contents and the same emptiness gave the same answer, while the name promised a filter. Nothing misbehaved; the signature described something the body never did. This test is the *smaller* half of the guard and says so. The real guard is the signature: the defect the ticket describes is no longer constructible, exactly as `Checked`'s constructor is the guard for the basis check rather than a test that can fail. What this test does add is that `Aggregation::of` is the only place the list is read, and that a list of two hidden dimensions is not somehow a third state — the reduction is emptiness, and the enum is not a count. ### apply_agg_over_nothing_is_absent_for_every_function An empty cell must read as *absent*, not as zero — otherwise every blank intersection in a pivot renders `0` and a sparse model looks fully populated. Fails if `apply_agg` ever gains a `Sum => 0.0` identity arm for the empty case. `eval_agg` deliberately has the opposite rule; see the next entry. ### eval_agg_over_no_rows_is_zero_only_where_an_identity_exists A *formula* aggregate over no rows is `0` for `SUM`/`COUNT`, which have an identity, and an error for `AVG`/`MIN`/`MAX`, which do not. This is deliberately **not** `apply_agg`'s rule, and the two are separate functions precisely so the difference is visible. improvise-kdp named both after they had been duplicated inline in two evaluators; the test pins the contract directly rather than only through formula eval. Its **vehicle changed** with improvise-s08t. It reached "no rows" through `Expr::Ref("Nonexistent")` — a name that resolves to nothing — which was precisely the bug s08t names: an unresolvable ref left the selection unconstrained instead of erroring. So the test was standing on the defect to demonstrate a neighbouring property. It now uses a name that **resolves** and has no cells (`Revenue`, declared and never written), which is the emptiness the entry is about; an unknown name is now an error and is pinned separately by `an_unknown_name_inside_an_aggregate_errors_for_every_function`. ### find_item_category_falls_back_to_formula_targets `add_formula` never inserts its target into `_Measure` (the item is *implied* by the formula definition). So `find_item_category` must fall back to the formula list, or `Expr::Ref("Profit")` fails to resolve and every cell of a chained formula errors `ref:Profit` (improvise-nrs). The test asserts the setup — that the target really is not a stored item — before asserting the lookup, so it cannot pass by the item having been added after all. ### stored_virtual_categories_match_their_names `CategoryKind::of_name` is used all over the codebase in place of looking the `Category` up. That substitution is only sound because (a) a fresh `Model` really does store each reserved name under its virtual kind and (b) `add_category` hands back the existing virtual category rather than shadowing a reserved name with a `Regular` one. The second half is the arm that would break silently: a shadowed `_Measure` would still answer `of_name` correctly while the stored category had the wrong kind (improvise-2lh). ### aggregating_one_text_value_yields_that_text The head of the improvise-l4qx family. `aggregate_cell_values` used to be `filter_map(as_f64)`, which **dropped** text. `_Index`/`_Dim` sit on `Axis::None` in every default view, so `none_cats` is non-empty almost always and the aggregating path was almost always taken — making **every text cell in every document render blank**. `.improv` carries pipe-quoted text as a first-class value, so a user could type a text value, save, load, and never see it again. ### aggregating_differing_text_values_is_an_explicit_error Two different words have no reduction. The user is told `mixed` rather than shown one of them arbitrarily, or a blank. ### aggregating_text_among_numbers_is_an_explicit_error The case that produced a *plausible* number before: the text was dropped and the rest summed, so the cell read as authoritative while under-reporting. A blank would have been noticed; a wrong total is not. ### a_lone_number_still_counts_as_one_rather_than_passing_through The regression guard for the hot path, and the test that pins the **branch order** in `aggregate_cell_values`. The all-numeric branch is checked *first*, which 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 this test sets `measure_agg["Status"] = Count` and requires `1`, not `7`. Reordering the two branches passes every other test in the family. ### aggregating_nothing_is_still_absent The text rule must not turn `apply_agg`'s emptiness rule into an error or a zero. Fails if `aggregate_cell_values` answers on an empty slice at all. ### where_filter_absent_category_does_not_apply_formula `Profit = 42 WHERE Region = "East"` evaluated against a key with **no** Region coordinate must not apply the formula: the condition cannot be satisfied, so the raw cell value (absent) is the answer. The bug was an `if let Some(item_val)` that failed to bind when the category was absent and fell through, applying the formula unconditionally — so a grand-total cell showed the filtered formula's value. ### sum_inner_expression_constrains_which_cells_are_summed `SUM(Revenue)` ignored its inner expression and summed every numeric cell matching the partial key. With `Revenue=100` and `Cost=50` both stored for `Region=East`, `Total = SUM(Revenue)` returned `150`. The fixture needs **two** measures on one coordinate or the bug is invisible. ### add_formula_same_target_name_different_category_both_coexist `add_formula` deduplicated by `target` name alone, ignoring `target_category`, so `Profit` in `_Measure` and `Profit` in `KPI` could not coexist — the second silently replaced the first. ### add_formula_same_target_name_different_category_evaluates_independently The consequence of the same bug seen through evaluation rather than through `formulas.len()`: the dropped formula's cell evaluated to absent. Worth keeping beside the count assertion because a fix that deduplicates correctly but dispatches on name alone would pass the first and fail this one. ### remove_formula_only_removes_specified_target_category The mirror image: `remove_formula` matched by target name alone, so removing `Profit` from `_Measure` also destroyed the `Profit` formula in `KPI`. ### near_zero_divisor_consistent_with_equality_epsilon improvise-0bf. `=`/`!=` compare with `FLOAT_EQ_EPSILON = 1e-10`, but the division-by-zero guard checked `rv == 0.0` exactly. With `X = 1e-11` the comparison `X = 0` was true *and* `100 / X` happily returned `1e13` — incoherent semantics for `IF(X = 0, a, y / X)`, whose else branch divides by a value its own condition called zero. Any value the equality operator treats as zero must also trigger `div/0`. This exercises the **direct** eval path (`eval_expr` inside `eval_formula_depth`). ### near_zero_divisor_consistent_in_cached_eval_path The same bug in the **fixed-point/cached** path (`eval_expr_cached` inside `eval_formula_with_cache`), whose division guard also used exact `== 0.0`. Two tests because there are two evaluators and a rule stated in one and not the other is a live bug — which is exactly how this one shipped. ### recompute_formulas_multi_stem_values_stable improvise-6os replaced an O(n²) `Vec::contains` dedupe of coordinate stems with an `IndexSet` — a pure performance change. The test pins the observable contract instead of the mechanism: a multi-stem model (chained formulas, several measures per stem) produces the stated values, and a second `recompute_formulas` call reproduces the **exact same cache**, so a dedupe that lost or duplicated stems would show up as a changed value or a non-idempotent cache. ### formula_chain_preserves_full_precision Display rounds; eval must not. `Tax = Price * 0.075` on `Price = 10` is exactly `0.75`, which `format_f64(_, true, 0)` displays as `"1"`. The test asserts both: that the display really does round (so the fixture is discriminating) and that `Total = Price + Tax` is `10.75` and not `11`. Without the first assertion the test would pass on a fixture where rounding changes nothing. ## `format.rs` ### a_finite_number_never_displays_as_infinity improvise-pikk. `round_half_away` scales by `10^decimals` before truncating, so `n * factor` overflows to infinity for a large finite `n` and any nonzero precision: `format_f64(1e307, false, 2)` rendered `"inf"`. The model held the number correctly and the display invented an infinity. Gotcha 8 is why nothing else caught it — `format_f64` is the whole display path and never feeds back into eval, so this is a wrong pixel with no wrong number behind it to trip a formula test. Reachability is low and that is not a reason to leave it: the shipped default format is `",.0"`, whose factor is `1`, so it needs both an absurd magnitude and an explicit `.N` format. The test carries its own anti-clamp assertion, because "return `n` unchanged" satisfies the overflow case *and* every other case. `1e307 * 100` overflows and `1e306 * 100` does not, which pins the threshold; and `format_f64(2.5, false, 0) == "3"` pins that the rounding path is still reached at all — half-away-from- zero, where a passthrough would hand the value to `{:.0}` and get Rust's half-to-even `"2"`. Inject the fault by dropping the `is_finite` arm. ## `model/cell.rs` ### matching_cells_yields_the_primary_insertion_order improvise-shyt. `matching_cells` walked the secondary index's `HashSet`, so its order was hash order rather than entry order. `DataStore::cells` is an `IndexMap` *precisely so* records mode can list rows in entry order. The check is against `iter_cells` — the primary map's own order — and **not** against a literal, which would only pin whatever the hasher happened to do on this machine today. ### the_same_cells_entered_in_opposite_orders_list_in_opposite_orders The half a single-process test cannot reach by calling twice: two stores holding the *same* logical cells, built by opposite insertion histories. Each must list its own history, so one result is the reverse of the other. Under the index walk the two orders were unrelated to both histories **and to each other**, because `RandomState` keys each fresh `HashMap` differently. `doctrine(an_order_test_needs_two_histories)`. ### matching_values_yields_the_primary_insertion_order The same guarantee for the value-only hot path, which aggregation folds `f64` over. f64 addition is not associative, so an unstable order made a `SUM` reproducible only within one process — drift in the last bits, which is the kind of wrongness nobody reports and nobody can reproduce. `Min`/`Max`/`Count` and the text-unanimity rule are order-free and were never affected. ### an_empty_partial_matches_every_cell_from_either_door improvise-hkt6. `matching_positions` indexed `sets[0]` and panicked on an empty partial. Both public callers guard, so it was unreachable — but a guard one level away from the indexing is how this class of bug returns, so the input the guards prevent is fed in directly here. It asserts the **answer**, not merely the absence of a panic: an empty partial constrains nothing, so every cell matches, and that must be the same answer the callers' fast paths give. Returning `vec![]` would also have been total and would have made deleting a fast path silently flip "all" to "none". ### live_records_come_back_in_the_stores_own_insertion_order improvise-2yc2. `live_records` walked the `records: HashMap`, so it yielded a different order in every process and in every instance — the improvise-shyt defect, caught here before a caller could inherit it. The order decided on is the store's own: a record sits where its **first** cell sits in the primary `IndexMap`. The fixture interleaves the measures deliberately — East's second measure arrives after West, and must not move East behind it. ### the_same_records_entered_in_opposite_orders_list_in_opposite_orders `doctrine(an_order_test_needs_two_histories)` applied to records: one history cannot tell "ordered" from "hashed that way today, on this machine". ### a_recreated_record_takes_its_new_place_not_its_old_one The claim `live_records` makes about **why** it derives the order from `cells` rather than memoising a position on `RecordEntry` — and which nothing else could see (Copilot, PR 20). A memoised position passes all three order tests above and still puts a re-created record back at its **original** place while its cells sit at the end of the store. The entry survives the deletion on purpose (it is a tombstone, below), which is exactly what makes the memoised position available to be wrong. ### deleting_and_recreating_a_record_does_not_reset_its_version The tombstone earns its place here. If `RecordEntry` were dropped when the last cell went, a delete-and-recreate would reset the counter to zero — and a client holding version 0 against the *old* record would be told its basis is current and its write applied. That is precisely the silent accept ADR 001's basis check exists to prevent. ### a_surrogate_for_a_removed_record_fails_to_resolve The first arm of staleness: a surrogate for a record whose cells are all gone must fail to resolve rather than resolve to *some other* record. The fixture holds **two** records, so a bug returning "any record" is visible; with one, the assertion would pass by naming the only record there is. ### a_second_measure_on_a_record_resolves_to_the_same_record The records-mode unit seen from the index side: two measures on one coordinate tuple are one record, so they share one entry. A cell-level unit would be violated by one legal edit (renaming a category coordinate moves every measure cell in the row). ### a_live_records_surrogate_is_the_one_that_resolves_it The pair `live_records` yields is not two facts that could drift apart: the surrogate is the record's own, so it is the one that resolves it back. ## `model/record.rs` ### a_records_surrogate_is_pinned_to_a_literal The surrogate must mean the same thing in the process that minted it and in the one that restarts under it, so it is pinned to a literal rather than to "whatever this build computes". **A failure here is not a bug in the test**: it means the hash, the key or the byte serialization moved, and every surrogate any client holds is now a name for something else. That is a wire-format break. ### the_two_kinds_do_not_share_a_hash A record and an aggregate may be built from the same coordinates — an aggregate over a single record is the degenerate case. A client holding an aggregate id must not be able to spend it as a record id by accident, which is what the domain tag byte is for. ### coordinates_that_concatenate_alike_do_not_hash_alike The length prefixes in `hash_coords` earn their place: without them `("Ab","c")` and `("A","bc")` serialize to the same bytes, and a collision here is a silent write to the wrong record. ### renaming_either_half_of_a_coordinate_changes_the_hash Two categories, not one: a one-coordinate fixture cannot tell "the item moved" from "the category moved" — the same blind spot as the one-category fixture that could not distinguish `TransposeAxes` from `CycleAxis`. ### a_surrogate_round_trips_through_json_as_a_string `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** — resolving to a different record or to none. The test asserts the wire shape (a quoted 32-hex string, 34 bytes with quotes), not merely that a round trip succeeds, because a numeric encoding round-trips fine inside Rust. ### construction_order_does_not_reach_the_hash `CellKey` is canonically sorted and `without` preserves that order, so "the normalized record" is a property the type guarantees rather than a convention to maintain. ## `view/types.rs` ### view::types::tests::moving_a_category_off_the_constraining_axes_forgets_its_page_selection improvise-z336, measured under Chromium by cycling a category all the way round: `Pag(Espresso) -> None` (unfiltered, 63,700) `-> Row -> Col -> Page` and **Espresso is back**, unasked, at 18,400. `set_axis` never touched `page_selections`; only `on_category_removed` and `on_item_removed` ever cleared one. So every route out of a filter merely **hid** it and the selection was silently reinstated the moment the category returned to Page. What the report and the ticket both concluded — that the browser had no way to clear a filter — was true, and the mechanism was not the one either named. There is **no clear-a-page-selection operation anywhere**: not in `ModelCommand`, not in the `View` API, and **not in the TUI** either, so this was a product gap at parity rather than a browser regression; `[` and `]` cycle among the category's items with no "All" entry to land on. Making `set_axis` forget the selection closes it with no new command, no new wire variant and no new key: cycling the axis round is now genuinely a way out. Inject the fault by deleting the `page_selections.remove`. The test asserts **both** halves — that the selection is gone while off Page, and that it is still gone on the way back — because the first alone is satisfied by a `View` that merely stops *reporting* the selection while the category sits elsewhere, which is what the code already did. ### view::types::tests::moving_a_category_from_page_to_filter_keeps_its_page_selection The trap gotcha 27 exists to name, and the rival fix this separates out. The obvious spelling is "clear it unless the new axis is `Page`" — and `Filter` constrains too: a drill fixes its coordinates on `Filter`, not `Page`, and `.improv` round-trips a selection under it. That version passes the test above and silently breaks drill. The predicate is therefore `Axis::filters_data()`, which is `filtering_cats()`'s own membership — "what data is visible", never "what the user can act on". It passes before the fix as well as after: it is there to stop the next change. `roundtrip_tests.rs` would also have caught it, but only as a persistence failure a long way from the line that caused it. ### a_pivot_view_still_seats_categories_row_then_column_then_page improvise-2zq, the **negative arm**, and the load-bearing one. The fix makes a records view seat a new category on `Axis::None`; without this test, "always seat on `None`" would pass every assertion about records mode while quietly deleting the auto-assignment `on_category_added` exists for. Probing confirmed the asymmetry: seating every category 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. ### a_records_view_seats_a_new_category_on_none_and_stays_a_records_view `on_category_added` used to bump a virtual off Row or Column to make room. With `_Index` on Row and `_Dim` on Column that *is* records mode, so naming a category with `N` while in records mode silently ejected the user from the grid they were standing in and returned them to a pivot. ### a_measure_on_column_is_not_bumped_off_it The second thing the bump broke, and the reason the fix is about virtuals in general rather than about records mode: `_Measure` on Column is an ordinary configuration — it is what `web/demo.improv` ships — and adding a second regular category kicked it off the axis, making every measure vanish from the grid. The fixture is deliberately **not** a records layout, so it exercises the pivot path rather than the early return. ### cycle_axis_unknown_category_is_noop improvise-t8s: `cycle_axis` panicked through `axis_of`'s `expect` when given a category not registered with the view, while `set_axis` silently no-ops on the same input. It now goes through `try_axis_of`. ### tileable_cats_omits_filter_but_keeps_every_other_axis improvise-dz8: a coordinate the drill fixed must not appear in tile select — the user cannot move it, so offering it offers an action that cannot be taken. The test flips the category back to `Page` afterwards, so it distinguishes "omits `Filter`" from "omits this particular category". ### filtering_cats_covers_page_and_filter_but_not_the_rest `Filter` constrains the data exactly as `Page` does; only `Page` is navigable. The fixture puts a category on each of `Page`, `Filter`, `Row` and `None` so the result separates the two questions rather than merely counting. ### on_category_removed_forgets_a_collapsed_group improvise-qx1z arm 2. `View::on_category_removed` cleared `category_axes`, `page_selections` and `hidden_items` — but not `collapsed_groups`. The other three lines being right there is what makes this an omission rather than a decision: collapse `Month/Q1`, delete the category, re-add it with `Jan[Q1]`, and the group comes back **pre-collapsed** with nothing saying why. Reachable from shipped keys, through the category panel. Fault injected: delete the added line; the test fails at `is_group_collapsed("Month", "Q1")`. The test re-adds the category after removing it, because a view that merely *forgot the axis* would also report `false` for a category it no longer knows — asking the question after the re-add is what separates "cleared" from "not currently registered". ### on_item_removed_forgets_a_hidden_item improvise-qx1z arm 1, at the `View` level. `Model::remove_item` removes the item and its cells and **touches no view at all**, so `hidden_items["Region"]` kept `"East"`: re-adding the item put it in the model and in the category panel while leaving it absent from the grid, with no indication why. The fixture hides **two** items and removes one. Without the second, a fix that dropped the whole category entry — `hidden_items.remove(cat_name)` — would pass while destroying unrelated state; the surviving `West` assertion is the input that separates "forget this item" from "forget this category". ### on_item_removed_forgets_a_page_selection_naming_it The third question improvise-qx1z asked to check while there, and the answer was the same: a page selection naming a removed item survived it. Asserted in both directions in one test — removing a *different* item must leave the selection alone — because a hook that cleared the selection unconditionally would satisfy the removal half and silently reset the user's page on every unrelated delete. ## `view/layout.rs` ### prune_axis_items_indexes_data_positions_not_visual_ones `prune_axis_items` asks `keep` about **data** indices — the positions a `row`/`col` argument means — so a group header between two data items must not shift the numbering the predicate sees. The fixture places the header at visual index 1 and prunes data index 1: if the predicate saw visual indices, `"A"` would be the one dropped. improvise-n3wt also removed the `&[bool]` + `debug_assert` pair, whose length could disagree with the axis in release builds only. ### a_text_cell_renders_in_a_default_view The user-visible face of improvise-l4qx, asserted at the layout rather than at the aggregation helper. The test asserts its own premise first — `!layout.none_cats.is_empty()` — because if a default view ever stopped putting `_Index`/`_Dim` on `Axis::None` the test would pass without exercising the bug at all. ### records_mode_gives_each_measure_its_own_value_column Pins the whole column vocabulary in order: `[Region, +cat, Revenue, Cost, +]` — the regular categories, the new-category column (improvise-2zq, sitting where the column it creates will appear), one column per raw measure, then the new-measure column (improvise-c5v). It checks both the labels and the `RecordCol` variants, so a column drawn with the right label but resolved to the wrong kind fails. ### records_mode_initial_layout_is_sorted On entry into records mode the store is sorted by `CellKey`, so rows appear in a deterministic order regardless of insertion order. The fixture inserts `East` after `North`, the opposite of alphabetical, so "did nothing" fails. ### records_mode_new_record_appends_at_bottom The complement: once the layout exists, a new record appends rather than re-sorting into the middle. The new record's coordinate (`AAA`) sorts *before* every existing one, so a re-sort would move it to the top and move the row the user was looking at. ### records_display_out_of_bounds_col_returns_none improvise-byz: `records_display` for an out-of-bounds column called `col_label(col)`, which returns `""`; `""` was then looked up as a category name in the record, never found, and the function returned `Some("")` — masking view/data mismatches as blank cells. It must return `None`, symmetric with `cell_key`. ## `view/axis.rs` ### the_label_vocabularies_are_unchanged improvise-rml, characterization: these strings were scattered across four render/command sites before being pulled onto `Axis`. Pinned verbatim so the move could not quietly restyle the UI. ### filter_is_outside_the_rotation improvise-dz8: `Filter` names a coordinate the drill fixed, so it is outside the Row→Column→Page→None rotation entirely. Both directions are asserted — cycling a `Filter` must not hand the user a navigable axis, **and** no navigable axis may rotate into `Filter`, or tile select could hand one out. ### next_is_a_four_cycle_over_the_navigable_axes Applying `next` four times is the identity from every starting point and no navigable axis is a fixed point, so no variant is stranded or skipped. Together with the entry above this pins the rotation without enumerating it twice. ### a_routing_formula_over_a_group_sums_the_members_answers improvise-4zl7, and the hand-computed half of its evidence. Every number in `route_then_aggregate_tests` is written out rather than obtained from a second call into the evaluator, because the bug being guarded is *the evaluator answering consistently and wrongly*. The fixture is four records — `Cost` of 10 and 100 in North, 20 and 200 in South — and the formula is `Pred = SUM(IF(Cost < 50, 1, 10))`. `Cost` straddles the threshold **inside each region**, which is the whole design of the fixture: | cell | route-then-aggregate | aggregate-then-route | |---|---|---| | one record (`Cost` 10) | 1 | 1 | | North (`Tag` away) | 1 + 10 = **11** | `110 < 50` → **10** | | everything | 1+10+1+10 = **22** | `330 < 50` → **10** | A fixture whose records fall on the same side of the split would give the same answer either way at the record level and differ only by a factor of the record count, which is the kind of agreement that reads as correctness. Two faults were injected and both were watched to fail. Restoring the old `eval_agg` — sum the matching raw values and ignore the inner expression — fails **7 of the 8** tests with 330 for 22, 110 for 11 and 10 for 1; the one survivor is `is_is_one_when_the_cells_own_coordinate_matches_and_zero_otherwise`, which has no `SUM` and so never reaches the fold. Making `eval_is` return `0.0` unconditionally fails exactly the **3** coordinate tests, with 400 for 214 and 200 for 14. The two faults failing disjoint-but-overlapping sets is itself the evidence that the tests separate the fold from the predicate. ### a_coordinate_split_survives_aggregating_over_that_very_category The case the ruling exists for, and the one the ticket's own text called the product question. With `Region` aggregated away the cell has no `Region` coordinate of its own, so a formula that read the *cell's* coordinate would see nothing and take `Expr::Is`'s `unwrap_or(false)` branch — answering 100 once. The fold hands the expression a **record** key, so each of the four records answers for its own coordinate: 7 + 7 + 100 + 100 = 214. ### a_member_whose_input_is_missing_errors_rather_than_being_dropped The asymmetry between the `Expr::Ref` fast path and the fold, asserted rather than left to be discovered. `SUM(Revenue)` skips a record with no `Revenue` cell; `SUM()` cannot, because the expression is what is being asked and an expression that will not evaluate is an error. The fixture clears one record's `Cost` and writes a different measure on the same record, so the record still exists and still has a member cell to be found by — otherwise the record would simply vanish from `matching_cells` and the test would prove nothing. ### feature_roles_line_up_with_the_names_preprocess_emitted Lives in `gp-tree`, recorded here because the rule it guards is improvise-4zl7's. `feature_roles` is the one statement of `preprocess`'s expansion order and `encode_row` drives off it, so a drift is silent in both directions: the emitter would spell the wrong feature and a re-encoded row would land in the wrong column. The assertion rebuilds each feature's *name* from its role and compares it with the name `preprocess` emitted, which is the only thing that ties the two walks together. The last four assertions exist because the loop is vacuous over a fixture that produces only one kind of feature. The fixture has a numeric column with a missing cell, a categorical column and a date column, so all four roles appear. Fault injected: swapping the `Numeric` and `Missing` pushes in `feature_roles` fails this test **and** `encode_row_replays_the_training_expansion`, which is the pair that would have disagreed. ### a_formula_targeting_another_category_still_folds_over_its_records Found by self-review, not by the ticket. The fold evaluates the inner expression at each member record's key, and the first version built that key by folding the record's coordinates onto **`context`** — which still carries the formula's own target coordinate. For the ordinary `_Measure` target that is harmless, because `Expr::Ref` overwrites `_Measure` on the way in. For a formula targeting any other category it is not: the record key keeps `Levy=Tax`, so `evaluate_depth` matches the formula again at that key and the whole cell converges to `Error("circular")` — a formula quietly self-referencing through its own aggregate. The key is now folded onto `partial` (`context.without(target_category)`, plus any inline `WHERE`), which drops the target coordinate by construction rather than by every caller remembering to. Fault injected by restoring the `context` fold: this test fails with `Error("circular")` against `11.0` and the other eight pass, which is why it is a separate test and not an extra assertion on one of them. ### the_two_evaluators_agree_when_the_fold_reads_a_derived_measure Adversarial review of PR #54, and the sharpest finding of the round: the first version of the fold reintroduced **exactly the divergence the ruling exists to remove**, through the cached path. `eval_expr_cached`'s `Expr::Ref` looks the value up in `formula_cache` at `context.with(cat, name)`. Inside the fold, `context` is a **member-record key** carrying the `none_cats` coordinates, while `formula_cache` is keyed by stems with `none_cats` **stripped**. The lookup could never hit, fell through to `aggregate_raw`, found no raw cell behind a formula target, and answered `Err("ref:…")`. So a routing formula reading a *derived* measure gave `Number(11.0)` uncached and `Error("ref:Margin")` cached — and the cached path is the one a pivot grid runs whenever any category sits on `Axis::None`. The fixture is the four-record one plus `Margin = Cost * 2`, so the fold's input is a formula rather than raw data. The test asserts the uncached value **and** that the two agree; asserting only agreement would pass if both broke. The fix makes them agree by construction rather than by this test: `eval_expr` is hoisted to module level and **both** evaluators hand it to `eval_agg` as `eval_at`. Note the repair that was *not* taken — teaching the fold to hit the cache by stripping `none_cats` off the member key returns the **group's** value to every member, which is aggregate-then-route through a cache lookup, silent. Fault injected by restoring `eval_expr_cached` as the fold's evaluator: this test fails with `Error("ref:Margin")` and the other eleven pass. ### the_cached_evaluator_has_its_own_working_coordinate_predicate The same review's second finding, and a lesson about where to inject a fault. `Expr::Is` has an arm in **each** evaluator. The original evidence injected the fault into the shared `eval_is`, which both arms call — proving the tests see the predicate, not that they see the cached *copy*. Replacing only the cached arm with `Ok(0.0)` left the suite at 1330 passing. The first attempt to close it — a coordinate split run through `recompute_formulas` + `evaluate_aggregated` — **also** stayed green, and that is the interesting part: once the fold routes through `eval_expr`, an `Is` inside a `SUM` never reaches the cached arm at all. The cached arm is now reachable only by a **bare** `Is` outside an aggregate, which is what this test uses. Fault injected into that arm alone: it fails, `Number(0.0)` against `Number(1.0)`. The general shape: an injected fault proves coverage only of the code path it is injected into. Two arms of the same rule need two injections, and if one of them turns out to be unreachable, that is a fact about the design worth writing down. ### a_user_category_whose_name_leads_with_an_underscore_is_seated_as_regular improvise-bidq. `View::on_category_added`'s early return read `cat_name.starts_with('_') || self.is_records_layout()` — the **same expression** mixing two derivations one operator apart, the left a name prefix and the right `CategoryKind`. Four lines up, `is_records_layout` asks the enum. `Model::add_category` does not validate names, so a user category literally named `_Foo` was `CategoryKind::Regular` to `records_columns`, `cell_id` and `is_empty_model`, and "virtual" to these two lines — seated on `Axis::None`, so it never appeared in a pivot while every other consumer treated it as ordinary. The second site (`holds_a_regular`) had the same split, which is why the test asserts a *second* category's seat too: fixing only the first leaves `_Foo` on Row while `Region` also takes Row. Decided explicitly, per the ticket: `add_category` is **not** made to refuse a leading underscore. The three reserved names are the ones `CategoryKind::of_name` knows, `_Foo` is a legal document, and a user-visible restriction is a larger change than making one answer out of two. The fix is that both sites now ask `CategoryKind::of_name(..).is_virtual()`. Fault injected by restoring either `starts_with('_')`: the test fails, `None` against `Row` on the first site and `Row` against `Column` on the second. ### an_ungrouped_item_after_a_grouped_run_belongs_to_no_group ### a_grouped_run_does_not_leak_into_the_next_prefix improvise-gd83, the two arms of one defect. `expand_category` emitted a `GroupHeader` on **entering** a group and no boundary on leaving one, so `row_group_for`/`col_group_for`'s nearest-preceding walk answered with a group that had already ended. The arms are separate because the two escapes are different and a fix can close one and miss the other. The first is *within* one expansion — `Jan[Q1]`, `Feb[Q1]`, `Sep` — closed by emitting `AxisEntry::GroupEnd` on a `Some -> None` transition. The second is *across* expansions: `cross_product` expands the inner category once per prefix, each call starting with `last_group = None`, so `Food/Bob[Family]` is immediately followed by `Clothing/Carol`, which is ungrouped and inherited Family with no transition anywhere to hook onto. That one needs a `GroupEnd` at the **end** of an expansion whose last item was grouped. `GroupEnd` is deliberately NOT emitted on `Some(A) -> Some(B)`: B's own header already ends A's run, and emitting one there moved `data_row_to_visual_skips_headers` and `data_col_to_visual_skips_headers` for no gain. Those two are untouched, which is the evidence the boundary is as narrow as it claims. Fault injected by deleting either emission: the corresponding test fails with `Some(("Month", "Q1"))` / `Some(("Recipient", "Family"))` against `None`, and `the_two_front_ends_draw_the_same_column_header` fails with it. ### a_collapsed_groups_header_survives_pruning ### a_header_whose_items_all_prune_away_is_dropped ### a_partly_pruned_group_keeps_the_header_its_survivors_sit_under ### an_ungrouped_item_after_a_pruned_group_belongs_to_no_group improvise-lhi4 and improvise-u6gc: two defects in one `match` block, and they pull in opposite directions, which is why they were fixed together. The naive repair of the first — stop pruning headers — deepens the second. `prune_axis_items` kept a header only if a data item under it survived the predicate, and dropped a `GroupEnd` whenever the header it closed was still pending. Both readings are wrong about a case the entry stream can distinguish. **lhi4**: a collapsed group contributes **zero** `DataItem`s — `expand_category` emits the `GroupHeader` and then `continue`s past every item — so its header was always pruned and the group vanished from the grid entirely. `z` (toggle-group-under-cursor) resolves the group from `row_group_for(cursor)`, so with no header and no items no row named it: collapse became one-way from the grid, recoverable only through `:toggle-group `. **u6gc**: dropping a pruned header's `GroupEnd` **re-opens an earlier run**. With `Jan[Q1], Feb[Q1], Mar[Q2], Sep` and `Mar` empty, the kept list became `[H(Q1), Jan, Feb, Sep]` and the nearest-preceding walk answered `Q1` for `Sep` — exactly the inheritance improvise-gd83 added `GroupEnd` to prevent, so the regression was live whenever `prune_empty` was on. The fix distinguishes the two emptinesses **without consulting the `View`**, which `GridLayout` has already dropped: a run that contained no `DataItem` at all was never expanded, i.e. collapsed, and its header survives; a run that contained items the predicate rejected is genuinely empty and goes. **Superseded twice** (see `an_outer_group_whose_rows_all_prune_loses_its_header`). Since improvise-6qet a collapsed group emits its fold as a `DataItem`, so no flat run is item-less. The one run that still was, an outer group's header followed directly by an inner group's header, was kept by that arm when it should have gone (improvise-b5zh). The arm is deleted. A `GroupEnd` is kept whenever a run is open in the output, so a re-opened earlier header is closed even though the header between them was pruned. That keeps gotcha 1's shape — still a predicate over the data index, no `&View` parameter, no flag vector. The tests are separate because each is satisfied by a fix that misses another. `a_header_whose_items_all_prune_away_is_dropped` **passes before the change** and is the discriminator: it is what stops "keep every header" from counting as a fix for lhi4, and it is the acceptance criterion lhi4 names. `a_partly_pruned_group_keeps_the_header_its_survivors_sit_under` was added by self-review and covers the **commonest** case, which the other three do not reach: a group where some items prune and some do not. All three of the others put a group in an all-or-nothing state, so between them they said nothing about the mixed one — and it is the case a user meets first. Reviewing it turned up a fact worth keeping about the shape of the code rather than a defect. Rewriting `run_had_item` to mean "some item was **rejected**" instead of "there was an item" is a **no-op**, and the whole workspace stays green: the header is pushed eagerly by the *first surviving item*, so `pending_header` is already `None` by the time any rejection can be recorded, and the flag is therefore only ever read in runs where **nothing** survived — where "no item at all" and "no item accepted" coincide. The eager push is what makes the collapsed/pruned distinction robust rather than delicate, and a reviewer trying to break it by attacking that flag is attacking the wrong half. `a_collapsed_groups_header_survives_pruning` asserts the **whole** `row_items` sequence rather than `any(|e| is Q1 header)`. An `any()` cannot see a header emitted in the wrong position or an extra entry beside it; the sequence can. Note what it still cannot claim: two consecutive `GroupEnd`s are not reachable, because `expand_category` never emits them, so "no stray `GroupEnd`" is a structural property of the writer and not something any fixture falsifies. Its fixture carries an **unfunded `Mar`** for a reason integration review found: the first version funded every item, so nothing was ever pruned and the test **passed with `prune_empty` disabled entirely** — the name claimed more than the test could see, which is the same vacuity class as the `any()` above. With `Mar` present, turning pruning off leaves `DataItem(["Mar"])` in the sequence and the assertion fails, so the fixture now demonstrates that pruning ran. Three faults injected against the fixed code, measured over all four tests (`c` = collapsed, `p` = prune-away, `g` = GroupEnd, `m` = partly-pruned; `✗` = fails, which is the point): | injected fault | c | p | g | m | |---|---|---|---|---| | stop keeping the `GroupEnd` (drop the `run_open` push) | ✗ | ok | ✗ | ✗ | | drop `!had_item` — keep every pending header | ok | ✗ | ok | ok | | invert to `had_item` — keep only fully-pruned headers | ✗ | ✗ | ok | ok | Every column is distinct, so no test here stands in for another. In particular the **third row separates the collapsed test from the `GroupEnd` test** — integration review reported that no fault did, which does not reproduce against this tree; inverting the condition fails `c` while `g` passes, because a re-opened run is closed by the `GroupEnd` arm whatever the header condition says. The two membership failures read `Some(("Month", "Q1"))` against `None`. Funding is **per fixture and deliberate in both directions**. The three all-or-nothing fixtures fund every item they expect to survive, because on a wholly unfunded fixture every row prunes and the tests would assert over an empty grid — `row_count()` is asserted first in each for that reason. The collapsed fixture instead carries one *unfunded* `Mar`, so that pruning is demonstrably running: see its own note above. ### a_trailing_conversion_letter_does_not_erase_the_precision improvise-711n. `parse_number_format` read the precision as `fmt[i+1..].parse::().ok().unwrap_or(0)` over everything after the last `.`, so any trailing conversion letter made the whole suffix unparseable and the format fell back to **zero decimals, silently**. `format: ,.2f` — the spelling `context/repo-map.md`'s own `.improv` example carries, and the printf/Python convention a user reaches for first — displayed integers everywhere. Fails before the fix with `left: (true, 0)`, `right: (true, 2)`. The test also pins the reading against its nearest *wrong* neighbour. Two implementations both fix `,.2f`: take the leading digit run, or trim the trailing non-digits off the tail. They differ only on a tail with digits on both sides of a letter, so `".2f3"` is in the assertion. Injecting `s.trim_end_matches(|c: char| !c.is_ascii_digit())` in place of `leading_digits` fails the test at `left: (false, 0)`, `right: (false, 2)` — so the test sees the rule, not merely the bug. ### a_zero_padded_picture_string_is_read_as_printf_not_as_excel The companion to the above, and the reason the fix is not "be generous". A forgiving reader would have to answer `.00`, where printf says precision zero and an Excel picture says two places. This pins the printf reading — `"0.00"` is `(false, 0)` — so that "accept the trailing `f`" cannot quietly grow into "accept anything and guess". It passes before the fix as well as after: it is there to stop the *next* change, not to catch this one. ### a_precision_too_large_to_name_reads_as_none `".300"` does not fit `u8`, so it reads as no precision rather than as 300 decimal places. Unchanged by improvise-711n and pinned so the fallback is a stated rule rather than an artefact of the integer type. ### a_format_naming_no_precision_at_all_reads_as_zero `",f"` has no `.`, and `".f"` has a `.` with no digits after it. Both are zero decimals. Pinned because `leading_digits` returns `""` for the second, and an empty string reaching `parse::()` is the one path in the function with no digits to carry the answer. --- ## `workbook.rs` ### workbook::tests::removing_a_measure_row_that_is_only_a_formula_target_removes_the_formula improvise-g3no through the facade both front ends actually reach — `effect::RemoveItem` calls `Workbook::remove_item`, never `Model::remove_item` — so this is the one that says the fix arrives where the keypress lands. Its leaf name is shared with the `model/types.rs` test below, hence the qualification. It asserts `effective_item_names("_Measure")` **by value** rather than `!contains("Profit")`, so a fix that removed the formula and also lost `Revenue` or `Cost` on the way fails here rather than passing a negative. --- ## `format.rs` — the decimal default and a formula's own format (improvise-e0ex) ### a_ratio_keeps_its_decimals_under_a_fresh_views_format The bug the ticket reports, at the smallest place it is visible. `View::new`'s `number_format` was `,.0` — *zero* decimals — so `2/3` reached `format_f64` and came back as `"1"`, half-away-from-zero. Written before the fix and observed failing with `left: "1" right: "0.67"`. It asks the **view's default** rather than a literal `",.2"`, so the assertion is about the shipped default and not about `parse_number_format`, which already had its own tests. Reverting `View::new` to `,.0` fails it. What it deliberately does not assert: that the default is exactly two places. `0.67` would also fail under `.1` or `.3`, which is the point — the test names "a ratio keeps its decimals", and the precise number is a product decision recorded in `context/code/core.md`, not a property. ### a_formulas_own_format_wins_for_its_target_and_nowhere_else The override, and both halves of "nowhere else" — a raw measure beside the formula target, and a key with no `_Measure` coordinate at all (the aggregating cell). The fixture uses `.4` against a `,.2` view so the two answers differ in *digits*, not merely in grouping. Two faults injected, in opposite directions, because one alone is not enough: - `FormatPlan::at` returning `self.view` unconditionally (the override never applies) fails with `left: "0.67" right: "0.6667"`. - `FormatPlan::at` returning the first entry in the plan for any key (the override leaks to every cell) fails with `left: "0.6667" right: "0.67"`. A test asserting only the target's own column passes the second fault, which is why the two neighbours are in the same test rather than in a sibling that could be deleted on its own. ### a_formula_naming_no_format_leaves_its_target_on_the_views The fallback, asserted where it is easiest to lose: a formula target whose formula carries no format must be indistinguishable from a raw measure. It keeps `FormatPlan::of` from indexing every target with some default and then "overriding" it back, which would be a second copy of the view's answer. ### the_formula_that_owns_a_cell_names_its_format_even_when_it_names_none Found by review (ChatGPT Codex, PR #125), not by the tests above, which is the entry's point: they used one formula and so could not see an ownership question at all. A target category need not be `_Measure` — `- Tax = Revenue * 0.1 [CustomCat]` is in the format's own documentation — so a key may carry coordinates two formulas claim. The first `FormatPlan` indexed **only the formatted** formulas, so a later formula's `FORMAT` reached a cell whose value an earlier, unformatted formula produces: the number and its spelling came from different formulas. The fixture puts the same two formulas in both orders and asserts the owner's answer each time, so a plan that prefers the formatted one fails the first half and a plan that ignores formats entirely fails the second. Fault injected: `.filter(|t| t.format.is_some())` before the `find` — precisely the behaviour of the version review rejected. Fails with `left: "0.6667" right: "0.67"`. ### the_owner_is_the_one_model_order_names_and_not_the_one_a_hash_does The other half of the same finding, and the one a single run cannot see. `by_target` was a `HashMap>` and `at` iterated it, so with two claiming categories the answer came from whichever hashed first — **a different answer per process**, gotcha 18's shape exactly (an index whose values are hashed is for lookup, never for choosing between candidates). It rebuilds the plan five times rather than asserting once, because each `HashMap` gets its own `RandomState` and a single construction can be right by luck. It is the same shape as `analyze_records`' five-iteration guard. This one cannot be made to fail by re-introducing a `HashMap` *reliably*, which is why the test that carries the load is its sibling above; kept because the repetition is what documents that hash order was the defect. ### a_view_created_after_a_label_still_keeps_it_off_the_axes `View::on_category_added` took a name and asked `CategoryKind::of_name`, which reads `Label` as `Regular` by design. `Workbook::add_label_category` papered over that by calling `set_axis(name, Axis::None)` straight afterwards, so the views existing *at that moment* were right; `Workbook::create_view` had only names to hand and had no such correction, so every view created after the label pivoted it. Reproduced at `d46ec59`: `Description` on `Axis::Row` in a view created one line after `add_label_category`. The fix gives seating the kind, and the two `set_axis` corrections are deleted — one answer rather than two, of which only one caller knew to write the second. ### a_view_created_after_a_regular_category_still_seats_it_on_row The control, and the direction the bug's own shape cannot supply. Seating everything on `Axis::None` satisfies the label assertion above completely, so without this a blanket `unseated = true` reads as a fix. Fault injected: `let unseated = true;`. This test fails (`left: None right: Row`) along with `a_label_does_not_displace_a_regular_category_in_a_new_view` and `first_category_goes_to_row_second_to_column_rest_to_page`, while every label assertion in the repository still passes. ### a_label_does_not_displace_a_regular_category_in_a_new_view The consequence the per-category assertions cannot state, because seating is **order-dependent**: the first regular category takes Row, the second Column, the rest Page. A label wrongly counted as regular therefore does not merely sit in the wrong place — it *consumes the slot* the next real category wanted, and on an already-pivoted document it pushes that category to `Page`, which filters the whole document down to one value. The fixture adds the label first and the regular category second, so the displacement is what is measured rather than the label's own axis. Fault injected: restoring `let unseated = CategoryKind::of_name(cat_name) .is_virtual() || self.is_records_layout();`. Five tests fail, including the two pre-existing ones (`add_label_category_sets_none_axis_on_all_views`, `label_category_defaults_to_none_axis`) that used to be carried by the deleted `set_axis` correction — which is the evidence that the consolidation did not cost coverage. --- ## `view/types.rs` ### view::types::tests::an_unused_view_name_steps_past_every_name_already_held improvise-vw47. `unused_view_name` is the one derivation both user-facing "create a view" resolvers call, and the whole of what it must get right is that the *count* of views is not a free *name*. Deleting `View 2` from `Default`, `View 2`, `View 3` leaves two views and the name `View 3` taken, so the old `format!("View {}", len + 1)` named a view that already existed and `Workbook::create_view` — which replaces, deliberately, for the `_Records` and `_Drill` rebuilders (`context/code/core.md` §11) — reset it to defaults. Four cases, and only the last three discriminate. `["Default"] -> "View 2"` and `["Default", "View 2"] -> "View 3"` pin the no-gap behaviour so the fix cannot be paid for by renaming the ordinary case; `["Default", "View 3"] -> "View 4"` is the bug; the run `["Default", "View 3", "View 4", "View 5"] -> "View 6"` separates "searches" from "bumps once", which a single-gap fixture cannot. The last case, `["Default", "Sales"] -> "View 3"`, pins that a name outside the `View ` family still counts toward the starting guess — the helper counts views, it does not parse names. Two rivals were injected and both fail here or one level up. Starting the search at `1` rather than at the count fails this test's first case **and** the pre-existing `create_and_switch_view_names_incrementally`: it renames the first view a fresh workbook creates. Making `Workbook::create_view` refuse or rename a name it already holds passes this test and breaks records mode, because both rebuilders depend on the replacement — which is why the fix is a helper beside `create_view` and not a change to it. ### model::cell::nonfinite_cell_values::a_nonfinite_number_survives_the_json_wire improvise-55v2. `serde_json` writes a non-finite `f64` as `null`, and `null` cannot be read back as one — so `CellValue::Number(f64::INFINITY)` serialized to `{"Number":null}` and the frame carrying it was unreadable at the far end. The websocket reader **skips** an unparseable frame and the worker reports only a console error, so one poisoned cell silently dropped a whole batch. `to_string` returned `Ok`, so nothing at the send site could notice either. **The design question the ticket left open is already answered by the format**, and measuring that was the useful half of this work. `.improv` 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, verified before anything was changed. So "refuse them before sending" would make a legal saved document unprojectable to a browser, and the remaining option is the one taken: give the wire an explicit representation. It also means `resolve.rs`'s `typed()` accepting `inf` is correct rather than the second half of a defect. The tokens are therefore **one home**, `nonfinite_token`/`parse_nonfinite` in `improvise-core`, called by the serde adaptor, by `persistence/format.rs` and by `persistence/parse.rs` — where three separate copies stood before and a fourth was about to be written. `format_f64` is deliberately not folded in: it is the display layer (gotcha 8) and may legitimately want a different glyph. Three rivals: - **A plain `derive` on the `f64`** — the state at HEAD — fails this test at `left: "{\"Number\":null}"` and also fails the io-side agreement test. - **Drifting the shared token** (`-inf` → `-infinity`) fails this test, the agreement test, and `generated_file_roundtrips`, because the writer and the reader stop agreeing. - **Re-copying the tokens into `persistence/format.rs`** so there are two homes again, drifted, leaves *this* test passing and fails the agreement test and the grammar round-trip. That is the rival the agreement test exists for: a second home that happens to agree is harmless, and one that does not is what must fail. The companion `an_ordinary_number_is_still_a_json_number` is the direction the fix could quietly break: it pins `{"Number":1.5}` exactly, and that a bare JSON integer still reads as a number rather than being mistaken for a token. ### persistence::nonfinite_agreement::a_nonfinite_cell_is_spelled_the_same_on_disk_and_on_the_wire improvise-55v2's other half. It writes each of the three values through `format_md`, reads the token off the `## Data` line, reads the token off `serde_json`, and requires them equal — then reparses and requires the value back. It is the guard that makes "one home" checkable rather than asserted: the two carriers are compared against each other, so a future second copy fails here exactly when it drifts, which is the only time it matters. ### model::types::formula_tests::count_aggregation improvise-a36c. The test asserted `val >= 1.0` for `COUNT(Sales)` at `Region=East`, where the fixture's answer is exactly `1.0` — so every inflated count cleared the bar. The ticket's own fault injection (`values.len() + 999.0` in `eval_agg`) is not the worst case: it fails one *other* test, `model::types::agg_helper_tests::eval_agg_over_no_rows_is_zero_only_where_an_identity_exists`, which sees the empty case move from `0` to `999`. Preserve the empty case (`if values.is_empty() { 0.0 } else { len + 999.0 }`) and the **entire workspace of 1699 tests passes** while `COUNT` over any non-empty set is wrong by 999. That measurement, not the ticket's, is what this entry exists to record: the only thing standing between a wrong `COUNT` and a green suite was a check on the identity element, which is the one input `COUNT` gets right by accident. Two assertions, because one does not separate the neighbouring question. `assert_eq!(at East, 1.0)` answers *is the count inflated* — it fails the `+999` rival at `left: Some(1000.0)`. It does **not** answer *does `COUNT` count anything*: a `Count` arm returning a constant `1.0` satisfies it, and at a fixture whose expected value is `1` that rival is invisible by construction. The second assertion, `COUNT` unscoped over all three regions being `3.0`, is what fails it, at `left: Some(1.0)`. Both rivals were injected and both fail, at different assertions — the first at the East count, the second at the total. The fixture is deliberately asymmetric: one matching cell under the scoped key and three under the unscoped one, so no single constant can satisfy both. ## `view/layout.rs`, `model/types.rs` — a collapsed group is one aggregate row (improvise-6qet) Reasoning in `context/code/core.md` §14. The bug is one statement: `expand_category` `continue`d past every item of a collapsed group, so the group had no data row and the cursor-addressed `z` could never name it. Restoring that `continue` against the fixed tree fails fifteen tests across `improvise-core`, `improvise-io` and `improvise-command` — every test below that touches the layout. ### a_collapsed_group_contributes_one_aggregate_row The inversion of `collapsed_group_has_header_but_no_data_items`, which asserted the premise 6qet changed and was rewritten rather than deleted. **Two** groups, one collapsed and one not, so `folds_a_group` is asserted `true` for the fold and `false` for `Apr` under the open group — a predicate that answered `true` for everything under a header would pass a one-group fixture. Also asserts the fold sits under its own header (`row_group_for(0) == Q1`), which is what lets `z` name it. ### a_non_contiguous_collapsed_group_folds_once `A[G1], B[G2], C[G1]`: a group's items need not be contiguous. Rival: dedup only against the *previous* group (right for the contiguous case, which every other fixture is) — `G1` then folds twice, under two headers, and only this test fails. ### a_category_nested_under_a_collapsed_group_is_folded_too `Month × Region` on Row, `Q1` collapsed: `Q1/North` is a fold too, because no record carries `Month=Q1` whatever sits beside it. Rival: the `collapsed` flag not inherited down `cross_product` — only this test fails, and without it the TUI would offer `Q1/North` for editing. ### model::types::model_tests::a_collapsed_groups_cell_aggregates_its_items `Jan=3, Feb=4` under `Q1`, `Apr=100` under `Q2`: the fold is `7`. That value separates the real fold from absent (`None`), from pass-through of the first member (`3`), from a count (`2`) and from a fold that leaks another group (`107`). Also the witness for the trap the first implementation of this ticket fell into: reading membership off `Category::groups` rather than each item's `group` field. `add_item_in_group` registers no `Group`, so that rival answers "not a group" for this fixture and the value is absent — this test, the next, and `a_collapsed_groups_header_survives_pruning` fail (the fold's row prunes as empty). ### model::types::model_tests::a_groups_stored_values_fold_once_not_per_member Under `Axis::None` (`Store`), `Avg` and `Count` over `Jan/A=2, Jan/B=4, Feb/A=9`. The group's average is `5` and its count `3`; folding the members' aggregates gives `avg(3, 9) = 6` and `count(2, 1) = 2`. Rival: always fold members' results (what the formula-target branch did before improvise-eccl) — this test fails. `Sum` cannot see the difference, which is why the fixture sets `measure_agg`. ### model::types::model_tests::an_item_named_like_a_group_is_read_as_the_item An item literally named `Q1` beside the group `Q1`: the item wins and reads `50`, so an existing document with such an item is unchanged. Rival: drop the `items.contains_key` short-circuit — the key expands to `Jan + Feb = 7` and this test fails. #### `a_collapsed_groups_header_survives_pruning`, re-aimed Re-aimed by improvise-6qet (the entry above, under improvise-lhi4, still holds for why the fixture carries an unfunded `Mar`). The expected sequence now includes the fold, `DataItem { ["Q1"], collapsed: true }`, and `row_group_for(0)` is `Q1` — the fold is the row `z` names the group from. Note the narrowing this implies: an **empty** collapsed group now has an empty fold row, which `prune_empty` drops, and its header goes with it. lhi4 kept that header because it was the only way back to the rows; the fold is now that way back. ### an_outer_group_whose_rows_all_prune_loses_its_header improvise-b5zh asked whether `close_pending_run`'s "a header whose run saw no item is collapsed, and survives" arm was dead since improvise-6qet, and proposed deleting it. It was **not dead**, and where it ran it was wrong. With two grouped categories on one axis (`Region` with `North[Upper]`, `South[Lower]`, outside `Month` with `Jan`, `Feb` in `Q1`), `cross_product` puts each outer header directly before an inner one: `[Upper, Q1, North/Jan, …]`. The inner header closed the outer header's run as "saw no item", so `Upper` was kept even with every North row pruned. At `1a6b877` the pruned axis was `[Upper, GroupEnd, Lower, Q1, South/Jan, South/Feb]`, an orphan header drawn over nothing. The fix keeps the pending headers as a stack: a header closes the pending header of its **own** category and everything nested inside it, a kept item flushes the whole stack, and a `GroupEnd` drops the innermost. Rival: a header clears every pending header, which is the flat reading of "a header ends the previous run". Only this test fails, because `Lower` is lost when `Q1` arrives. The flat-case tests (lhi4, u6gc, the partly-pruned group) pass unchanged under both versions, which is why none of them could see this. One pre-existing shape is left alone and stated: two consecutive `GroupEnd`s (an inner and an outer run ending together) keep only the first, because `run_open` is a single flag. That was true before this change, pruned or not. ### records_steps_are_the_spine_then_every_non_index_dim_category_on_none improvise-t0ma. Before this, `records_steps` was guarded by one incidental test: the `is_index_or_dim() → is_measure()` rival failed only `save_with_unfinished_record_is_readable`, a persistence test that happens to enter records mode. This asserts the **steps**, not the effects `cmd/grid.rs` translates them into, because the steps are the model-side answer a browser will also consume. The expected list filters `_Index`/`_Dim` by **name**, not by `CategoryKind`, so it is not a second copy of the predicate under test, and the setup asserts `_Measure` and a regular category are both present, since a model with neither could not tell the two predicates apart. Rival: `is_measure()` in place of `is_index_or_dim()`. This test fails, because `_Measure` is dropped and `_Index`/`_Dim` are put on `None` after the spine that just placed them. ### drill_steps_filter_each_coordinate_and_free_the_rest improvise-t0ma's companion for `drill_steps`. The fixture includes a `none_cat` that the key also fixes (`Region`) and one that is virtual (`_Index`), so both exclusions are exercised. It also lists the key's coordinates out of sorted order, which checks that the steps follow `CellKey`'s canonical order. Rival: `Axis::Page` in place of `Axis::Filter` for a fixed coordinate, which reopens improvise-dz8 (a drilled coordinate the user could page away from). This test fails. ### a_folds_ratio_formula_is_the_ratio_of_its_folded_inputs improvise-eccl. `Margin = (Revenue - Cost) / Revenue` with Jan `10/5` and Feb `30/3` under `Q1`, and Apr in `Q2` with no data, so the Q1 fold is row 1. The member ratios are 0.5 and 0.9, so the three candidate rules give three different answers: summed 1.4, averaged 0.7, pooled `(40 - 8) / 40` 0.8. The expected value is the **pooled** one, and it is also asserted equal to the same document with Month on `Axis::None`, which is the ticket's criterion. The None-axis document works because Apr carries no data, so the None axis pools exactly Q1's months. At `a164726f` the fold read **1.4**. The ticket's own fixture (0.5 and 0.5) would have separated sum from pool, but not mean from pool. The second formula set chains (`Profit = Revenue - Cost`, `Margin = Profit / Revenue`), so a reference to another formula target is read over the fold as well. A rule that folds a formula ref's members while pooling raw refs fails there. ### a_folds_aggregate_formula_pools_its_members_records The guard on the other side of improvise-eccl. Once a formula at a fold is evaluated at the group key, an aggregate inside it (`SUM`, `MAX`) must pool every covered member's records, because `Month=Q1` matches no record. `MAX(Revenue)` is the discriminating case. It **failed before the change too**: the old rule folded the members' `MAX`es with the measure's `Sum`, giving 40 where the answer is 30. `SUM` alone agrees under the old rule and the new one. Rival: `contexts_of` stops pooling at a fold (`Reach::Fold { .. } => Vec::new()`). This test alone fails. ### collapsing_a_group_with_a_hidden_member_leaves_the_total_where_it_was improvise-f2oo. `Q1 = {Jan 3, Feb 4}`, `Q2 = {Apr 100}`, Feb hidden. Expanded, the drawn rows total 103. At `a164726f` collapsing gave a Q1 fold of 7 and a total of **107**: `z` moved the total. The ruling: a fold covers the members the grid draws. The test asserts 103 both ways and a fold of 3. It then hides Jan too and asserts that there is no Q1 row and a total of 100. That half already held, and it is the inconsistency the ticket named: before, a hidden value appeared only while a sibling was visible. Rival: `GridLayout::evaluate` covering every member (`&|_, _| true`). This test alone fails, with 107. ### a_folds_aggregate_filtered_on_the_folded_category_counts_each_record_once Found by the ChatGPT Codex review of PR #203, at head `14acf46d`. Pooling an aggregate over a fold's member contexts ran `agg_values` once per member, and an inline `WHERE` on the **folded** category overwrites that coordinate, as it does at a record. Every member therefore became the same record set. `SUM(Revenue WHERE Month = "Jan")` on the Q1 fold read **20**, Jan twice. The test asserts 10, and equality with the Month-on-None document. The fix dedupes the member contexts after the filter is applied, so each record set is pooled once. The rival is the pre-fix code, which read 20. `a_folds_aggregate_formula_pools_its_members_records` could not see this, because none of its aggregates carried a filter. ### a_fold_evaluates_each_formula_it_reaches_once improvise-o1y0, from PR #203's review. A reference from a fold's formula to another formula target is evaluated over the fold in turn, with nothing remembered, so a doubling chain `F{i} = F{i-1} + F{i-1}` re-evaluated exponentially. The test **counts** formula evaluations rather than timing them, through a test-only thread-local counter. At `2a4f35d7` evaluating `F8` on one fold took **511** evaluations (2⁹ − 1); with the memo it takes 9. It also asserts the chain's value (7 × 256), so a memo that returned the wrong entry cannot pass. Rival: keep the in-progress marker but drop finished values from the memo (`.filter(Option::is_none)`). This test fails with 511. ### a_cycle_at_a_fold_is_still_circular improvise-o1y0's other half. `P = Q + 1`, `Q = P + 1` at a fold must read `Error("circular")`. The memo's in-progress marker is what answers it now, because the depth bound the fold path used before is gone. Without the marker the evaluation recurses until the stack runs out, which is why the marker is there rather than being an optimisation. ### one_formula_at_two_fold_keys_is_evaluated_at_each improvise-d7aj, found by the adversarial re-review of PR #203 at `b3379d1a`. The fold memo in `fold_formula_once` is keyed by the whole `CellKey`, and nothing pinned that: both tests above keep one key context throughout, so a memo keyed by the formula alone passed the whole workspace. A reference to an item of **another** category moves the key while `_Measure` names the same formula. Stores A (Jan 3, Feb 4) and B (Jan 100, Feb 200), `G = Revenue * 2`, `F = IF(Revenue > 50, G, G + B)`. At `(Q1, A, F)` Revenue is 7, so F is `G + B`: G at A is 14, and `B` reads F at `(Q1, B)`, where Revenue is 300, so that F is G at B, 600. The answer is **614**. Rival: the memo keyed by `formula.target` alone. F is still in progress at `(Q1, A)` when F at `(Q1, B)` is asked, so that key reads `circular`. Injected into a full workspace run, it fails this test and nothing else. The reviewer's probe, now committed. ### a_diamond_at_a_fold_is_not_circular improvise-d7aj's second case, which passed at `b3379d1a` and was pinned by nothing. `A1 = B2 + C3`, `B2 = D * 2`, `C3 = D * 3`, `D = Revenue` reaches D by two paths in one fold evaluation, so the second reach must find D *finished* and not *in progress*. It asserts **35** over D = 7 and exactly four evaluations, so a memo that re-evaluates D passes neither. Rival: never store a finished value, so only the in-progress marker stays. This test fails with `circular`, and so does `a_fold_evaluates_each_formula_it_reaches_once`, whose doubling chain is also a diamond at every step. So this test is a named pin of that shape rather than the only guard against it. ### a_fold_answers_is_and_where_on_its_category_as_the_none_axis_does improvise-lnkb, ruled by the orchestrator: on a fold, the folded category reads as absent. `Flag = Month IS "Q1"` and `Marked = Revenue * 2 WHERE Month = "Q1"` are each compared with the same document with Month on `Axis::None`. At `689c7665` `Flag` read **1** on the fold and 0 on None. The final assertion pins `Flag` at `Some(0.0)`, false rather than absent, so a fix that dropped the cell cannot pass. Rival: `read_coordinate` ignoring the reach (`if false && …`). This test fails. ### `is_item_agrees_with_effective_item_names_everywhere` **improvise-4daa**, found by the #265 review at `adb6b814`. `Model::first_non_item` used to ask `effective_item_names(cat).contains(name)` for every coordinate. That built and cloned the whole item list per coordinate per projected cell. `Model::is_item` asks the same question with no allocation. This test holds the two answers equal across every pairing of three categories (`Region`, `_Measure`, and one that does not exist) and six names: - an item; - a measure item; - a measure formula target; - a formula target on a regular category, which `add_formula` makes a real item; - a name that is nothing. A setup assertion checks that `Profit` is not in `_Measure`'s explicit items, so only the formula branch can answer it. **Rival:** `is_item` without the formula-target branch fails this test, and `ui::host::tests::a_write_under_a_formula_is_refused_for_the_formula_not_as_a_non_item` in `improvise-command`. **Bench**, not committed, on an Apple M4 Max, 16 cores. It is the reviewer's `bench265`, and each build ran interleaved 5 times; figures are the minimum per build. | build | pivot 150×120 (18k cells) | records 2000×5 (2000-item label) | |---|---|---| | `47b166ee` (#252, before) | 131 ms | 1.93 ms | | `adb6b814` (#265 head) | 193 ms | 145 ms | | with `is_item` | 136 ms | 2.82 ms | Single, non-interleaved runs on this machine varied by more than 2×. That is why each build was run 5 times, interleaved, and the minimum taken. ### records_mode_records_display_returns_values **What was vacuous (improvise-sfwy).** The category-column half asserted only `!display.is_empty()`, where the fixture's value is `North`. The ticket named the test under its old name, `records_mode_resolve_display_returns_values`. improvise-umi4 renamed it when `resolve_display` was removed, and kept the weak assertion. **Fault first, then the fix** (the improvise-jkxr procedure). Two faults were injected into `records_value`'s `RecordCol::Category` arm. The first returns `"GARBAGE"`. The second returns the category's *name*, `Region`, a plausible wrong answer. Under the old assertion the test **passed** with each fault. Now it asserts `Some("North")` exactly. With the faults it fails: `left: Some("GARBAGE")` and `left: Some("Region")`. Without them it passes. The production code is unchanged. ### pruning_keeps_every_surviving_rows_runs_at_every_level improvise-e265, strengthened by improvise-6u6s. On a two-level grouped axis, `prune_empty` used to change a surviving row's group. Pruning the inner header dropped its `GroupEnd` too, and under the flat reading the ungrouped inner item then joined the outer group. **What it asserts.** For every surviving row and at every level: - the run the row is in names the same `(category, group)` pruned as unpruned (the whole open-run stack, from `open_runs`, not only the innermost); - whether it is in the **same run** as the previous survivor is the same answer pruned as unpruned, where "the same run" means the same header entry. When every row survives, the pruned entries must equal the unpruned ones, marker for marker. **The fixtures** are five, each under every collapse subset of its groups: - `Region` over `Month`; - three levels, `Region` over `Month` over `Kind`; - `Month = [Jan in Q1, Mar, Feb in Q1]`, a non-contiguous group, so one parent holds two `Q1` runs (improvise-h0iv); - a middle level with no groups at all (`Kind` between `Region` and `Month`); - two outer groups side by side (`North` in `Upper`, `South` in `Lower`). **How it is built** (improvise-6u6s). Each fixture's layout is built once per collapse subset, with no cells, so every row is present. `prune_axis_items` is then called directly with a keep predicate for every non-empty set of rows. That makes 4,992 cases. The first version built two `Workbook`s per funded-cell mask, about 4,000 cases, and put `improvise-core`'s lib at 0.86–0.92s on an Apple M4 Max (16 cores) under load average 39–77, against about 0.16s without it. The reviewer measured 0.94–2.46s under heavier load, once past the 2s tripwire. Now the lib runs in 0.17–0.23s on the same machine, against 0.16–0.18s with the module skipped. `prune_empty` is still covered, because it calls the same pruner, and `prune_empty_keeps_a_row_past_an_inner_run_in_the_outer_group` pins that wiring. **Red and rivals.** The e265 form failed at base 202e7eac, on `mask 0b100` with only `North/Mar` funded: pruned said `Upper`, unpruned said `None`. Rivals, each injected and restored: - a flat reader (every marker read at level 0) fails it; - a pruner that owes no `GroupEnd` fails it (u6gc's shape: a run whose closing header was pruned is left open); - a pruner that keeps headers as they arrive fails it; - **runs keyed by `(level, category, group name)` instead of header position** fails it, at `fixture 2, mask 0b101, collapsed []: run identity at level 1, row 1`. Pruning `Mar` must leave North's two `Q1` runs as two, and the rival merges them. That last rival passed every test in #250 (improvise-6u6s). Those fixtures had no non-contiguous group and compared only the innermost group, so they could not see a merged run. The equality half was added after the first version of `remark` emitted `End0` where the input had `End1, End0`. That output was equivalent but not equal, and the assertion caught it. ### a_pruned_axis_draws_no_header_over_nothing_and_ends_on_no_marker improvise-e265's other property, over the same 4,992 cases. Every kept header must be followed, past any deeper headers, by a data item, and the axis must not end on a `GroupEnd`. At base 202e7eac, `mask 0b1` left a trailing `GroupEnd`. The ticket counted the header-over-nothing shape on main before #200. Rivals: - a pruner that keeps headers as they arrive fails the first half; - a pruner that passes `GroupEnd`s through as they arrive (the pre-e265 shape) fails the second. In the shipped pruner both properties hold by construction, because markers are emitted only before an item. So the obvious rival, deleting a trailing-marker strip, has nothing to delete. An earlier draft had that strip, and it was dead code. ### prune_empty_keeps_a_row_past_an_inner_run_in_the_outer_group The enumeration drives `prune_axis_items` directly, so this test pins that `prune_empty` reaches it. With only `North/Mar` funded, the pruned grid has one row, and it is in `Upper`. It is a wiring test, not a discriminating one: #200's pruner also answered `Upper` here. What it catches is a `prune_empty` that stops pruning (the row count) or a reader that loses the outer run. ### a_row_after_an_inner_groups_end_is_still_in_the_outer_group The user's ruling on improvise-e265, at the layout: membership is per level, and `row_group_for` answers the innermost open group. With nothing pruned or folded: - `North/Jan` is in `Month`/`Q1`, the innermost; - `North/Mar` is in `Region`/`Upper`, because `Q1` ended and `Upper` did not; - `South/Mar` is in nothing. Failed at base 202e7eac on the second assertion: the flat reading said `None`. The flat-membership rival fails it again. ### a_lookup_does_not_read_past_its_row improvise-0r01, which is ChatGPT Codex's P2 on #263. `innermost_group` built `open_runs` for the whole axis, cloning a level stack per row, to answer for one row on every `z` keypress. `open_runs` was made lazy and the lookup took `.nth(idx)`. That is superseded: the walk is now `walk_open_runs`, which lends one stack to a visitor rather than yielding a copy per row (see `a_late_lookup_answers_for_its_own_row`). The walk's extent cannot be seen through a well-formed axis, so the test builds an axis whose second header is unreadable on purpose: `level: usize::MAX`, which `Vec::resize` cannot allocate. A lookup for row 0 must answer `Upper` without reaching it. Rival: collecting `open_runs` before `.nth(idx)`, which is the eager shape. It fails with `capacity overflow`, and the other 335 core tests pass. That rival was run against the iterator form. Under `walk_open_runs` the equivalent is a visitor that never breaks, and the witness holds the same way. ### a_fold_row_evaluates_each_formula_once_across_the_row improvise-h6z1. `F0 = Revenue`, `F{i} = F{i-1} + F{i-1}` up to F8, as columns, with Q1 collapsed so the Q1 row is a fold. The test evaluates every cell of that row twice and compares the two runs: once with a fresh memo per cell (`evaluate`), where each column re-derives the chain below it (1 + 2 + … + 9 = **45**), and once with one memo shared across the row (`evaluate_in`), which gives **9**. It also asserts the two runs produce identical values, so a memo that returned a wrong entry cannot pass. The 45 is asserted as a setup, so a fixture whose chain had collapsed into one column could not pass the 9 by accident. Rival: `evaluate_covering_in` ignoring the memo it is given and making a fresh one. Injected into a full workspace run, it fails this test (45 against 9) and nothing else. ### a_spanned_scan_finds_what_a_lookup_per_member_finds improvise-yb4b. `fold_stored` used to enumerate one `CellKey` per member of a fold and look each one up. Since #239 a column total spans every row category, which is 3,000 lookups for one 18k column, paid twice per keypress. `DataStore::spanned_values` now answers with one index-narrowed scan. This test compares the scan with the per-member definition it replaces, in both modes, on a store built to separate them. It has a record with an **extra coordinate**: `get` misses it and `matching_values` finds it. It has an **uncovered item**, a record **lacking** the spanned category, **another measure**, and another value of an unspanned coordinate. It also has a covered item that was never interned. The values are distinct powers of two, so a sum identifies the admitted set. The order differs on purpose: the store's order, not member order, as gotcha 18 names it. Three faults were injected, each run against the full `improvise-core` and `improvise-command` suites: - Exact mode loses its length check (`==` → `>=`). **Only this test fails.** Nothing else in either suite reaches an `Exact` fold over a record with an extra coordinate. - The span ignores coverage (`|| true`). This test fails, and so do five others (collapsed-group, hidden-row and CSV-agreement tests). - A category the store never interned is skipped instead of matching nothing. **Only this test fails.** ### a_span_finds_what_its_members_find_in_store_order improvise-jq0m. `spanned_values` now chooses between two narrowings, and a fixed example can only ever take one of them. This property builds random stores over `R`, `P` and `M`. Some records lack `R`, some carry an extra `X`, and `r6` is a covered item the store never interned. It spans `R` and sometimes `P`, in both modes, and compares the result with the per-member definition, **in store order**: the values the members' own lookups find, read back in `iter_cells` order. Store and span sizes vary across cases, so both narrowings are taken. Three faults were injected, each run against the full `improvise-core` and `improvise-command` suites: - The per-item narrowing drops the base, so it keeps each item's pair alone. This test and `a_spanned_scan_finds_what_a_lookup_per_member_finds` fail. **Nothing in either crate's grid or command tests does.** Those fixtures never hold a second value of the base's coordinate within a small span. - The base narrowing drops the base. This test fails, along with eight fold and totals tests. - The per-item positions are not sorted after they are merged. **Only this test fails.** Values stay correct and only their order moves, which changes a Sum or Avg in the last bits. The regression itself is a cost, which no test here can see. The evidence for it is the `grid_projection` bench's grouped cases, added for this ticket. ### a_memo_answers_for_the_covers_it_was_built_with **What broke (improvise-pntu).** Found reviewing PR #247, and it was also ChatGPT Codex's P2 there. `FoldMemo` was keyed by (fold key, span categories), while `evaluate_covering_in` took the aggregation and the covers as separate arguments. With `margin_book(["R = Revenue / Cost"])`, key `{Month=Q1, _Measure=R}` and `Exact`, one memo was passed first with covers=all and then with Feb hidden. The second call returned **5.0** (40 / 8, the first fold's answer) where a fresh memo gives **2.0** (10 / 5, Jan alone). **Red first.** The test was first written against the old API, exactly as the reviewer gave the repro, and it failed `left: Some(Number(5.0)), right: Some(Number(2.0))`. The fix moves agg and covers into the memo (`FoldMemo::new(agg, covers)`), so that shape **no longer compiles**. The signature is the guard. The test now builds one memo per covers and asserts three things. Each memo gives its own fold's value. The Feb-hidden memo agrees with a fresh `evaluate_covering`. The covers=all memo still says 5.0 afterwards. **Rival:** `FoldMemo::new` storing `|_, _| true` in place of the covers it was given. This test fails, because Jan alone reads 5.0. ### a_memo_answers_for_the_aggregation_it_was_built_with pntu's other half. `{_Measure=Revenue}` names no month. So under an `Exact` memo it is not a stored cell (`None`), and under an `OverHidden` memo it sums Jan and Feb (`40.0`). **Rivals:** - `evaluate_covering_in` passing `Aggregation::Exact` instead of the memo's agg: this test fails. - The same fault in `evaluate_spanning_in`: caught by two existing tests, `a_max_measures_total_does_not_move_when_a_row_group_collapses` and `an_average_measures_total_is_the_mean_over_its_records`. So it needed no new test. ### pruning_evaluates_each_fold_formula_once_and_keeps_the_same_rows **What was slow (improvise-metf).** `GridLayout::prune_empty` evaluated every cell with its own fresh fold memo. That is h6z1's shape, and it sits on the same keypress path, since `with_frozen_records` prunes whenever the view asks. Each column of a folded chain re-derived the chain below it. **Measured first**, on an Apple M4 Max (64 GB) running other agents' builds. The bench is `cargo bench --bench grid_projection`, whose new "pruned" lines time a `with_frozen_records` build with `prune_empty` on. There were two back-to-back pairs, and the second put the old line back for the "before" half: | Case | Before (fresh memo per cell) | After (one memo per build) | |---|---|---| | dense 18k | 90 ms, 88 ms | 82 ms, 100 ms | | folded chain ~18k | 900 ms, 705 ms | 355 ms, 351 ms | | folded chain ~90k | 6.40 s, 6.62 s | 3.00 s, 4.96 s | It pays on the folded chain: about 2.0× to 2.5× at 18k, and 1.3× to 2.1× at 90k. The dense grid has no folds and moves only within noise. So `prune_empty` now shares one memo per build, taken from `GridLayout::fold_memo` and dropped before the axes are pruned. **The test.** It uses `margin_book`'s F0..F8 chain with Q1 collapsed. It counts fold-formula evaluations across one pruned `with_frozen_records` build and asserts **9**. It also asserts that the pruned rows are exactly the rows a fresh `evaluate` per cell would keep, so sharing the memo cannot change what is pruned. And it asserts that the Q1 fold row survives, so the count is not measured over an empty grid. **Rival:** the old line, `self.evaluate(model, &k)` per cell. This test fails: `left: 45, right: 9`. ### a_grid_evaluates_under_its_own_covers_not_another_grids **What was open (improvise-opbo, P3).** It confirms ChatGPT Codex's P2 on #258. `GridLayout::evaluate_in` and `column_total` took any `&FoldMemo`. With `margin_book(["R = Revenue / Cost"])` and Q1 collapsed, there are two layouts over one model: one draws Jan and Feb, the other hides Feb. The first layout's memo, handed to the second layout's `evaluate_in`, gave **5.0** for `{Month=Q1, _Measure=R}` where the second grid reads **2.0**. Nothing in production reached it, because each build made its own memo. **Red first.** The test was first written against `evaluate_in` with a foreign memo. It failed `left: Some(Number(5.0)), right: Some(Number(2.0))`. **Fix: the review's proposal.** `GridLayout::evaluator(model)` returns a `GridEval` that borrows the layout and owns a memo the layout built itself. It has `cell(key)` and `column_total(col)`. `evaluate_in`, the memo-taking `column_total`, and `fold_memo` are gone. So the shape the test first used no longer compiles, and the signature is the guard. The test now asserts two things. Each layout's evaluator answers for its own covers: 5.0 and 2.0. And asking one does not move the other. **Rival:** the evaluator's covers ignoring the layout's hidden items. This test fails `left: Some(Number(5.0)), right: Some(Number(2.0))`. **The earlier rivals, restated for the new API, still fail:** - `pruning_evaluates_each_fold_formula_once_and_keeps_the_same_rows` with a fresh evaluator per cell: `left: 45, right: 9`. - `one_grid_build_evaluates_each_fold_formula_once_for_cells_and_totals` with a fresh evaluator per cell: `left: 54, right: 18`. - The same test with a fresh evaluator per total: `left: 54, right: 18`. The entries above for h6z1, 0jw5 and metf name `evaluate_in` and `fold_memo` because those were the API when they were written. This entry supersedes the names, not the arguments. ### a_late_lookup_answers_for_its_own_row improvise-0r01, the second round: ChatGPT Codex's P3 on #273. The lazy `open_runs` iterator still yielded `Some(open.clone())` for every data item, so `innermost_group(..).nth(idx)` allocated and dropped a stack for every row before the target. A `z` near the end of a large axis paid nearly the whole original cost. `a_lookup_does_not_read_past_its_row` could not see this: it asks for row 0, and it proves only that the suffix is not read. The walk is now `walk_open_runs`, which lends one stack to a visitor and copies nothing. This test asks a late index. It covers rows 0 to 5 of a `Q1{Jan, Feb}, Mar, Q2{Apr, May}` axis, and index 5 is past the end. So it pins the row counting that the visitor form moved into `innermost_group`. Both rivals were run against the whole `improvise-core` suite: - A counter that never advances, so every lookup answers row 0: this test fails, along with four others. - A walker that ignores `GroupEnd`: this test fails, along with seven others. **What it cannot see is the allocation itself.** A visitor that clones the stack at every row returns the same answers, so no assertion here can tell the two apart without a counting allocator. The copy-free property is held by the shape: the visitor receives a borrowed slice of open runs (`&[OpenRun]` since improvise-felo.6), and no owned stack leaves the walk. It is not held by a test. ### a_parent_group_covers_every_leaf_beneath_its_child_groups improvise-felo.6. `items_in_group` read only the items whose own `group` named the group, so a year covered nothing: its months sit in quarters. Rival run against `improvise-core`: the direct-only filter restored. Four tests fail — this one, `a_hierarchy_opens_closed_as_one_fold_per_root` (the 2024 fold reads empty), `collapsing_a_parent_folds_its_children_whatever_their_state` and `hidden_items_leave_every_fold_above_them`. The fixture adds `2024-03` last, so the year and its first quarter are non-contiguous in insertion order. ### a_year_quarter_month_hierarchy_nests_its_runs improvise-felo.6: the whole marker sequence of a Year > Quarter > Month axis opened to depth 2, compared exactly. Rival: every header inside a category emitted at depth 0 (a flat hierarchy). This test fails, and so do `pruning_keeps_every_surviving_rows_runs_at_every_level` (the `NESTED` fixture), `the_nested_fixture_draws_runs_inside_one_category` and `pruning_drops_an_empty_inner_group_and_keeps_its_parent`. ### a_hierarchy_opens_closed_as_one_fold_per_root The user's ruling on improvise-felo.6: groups start closed. Two rivals fail it. The old open default (`inherits_collapsed` always false) fails twelve tests across `improvise-core` and `improvise-io`, this one among them. The INNERMOST collapsed group folding instead of the outermost (`.rev()` on the fold search) fails this one, `hidden_items_leave_every_fold_above_them` and `the_group_under_the_cursor_is_the_innermost_at_every_depth`: under it a closed year shows its quarters as rows. ### a_groups_depth_opens_it_and_n_minus_one_levels_beneath `expanded(2): Time/2024` opens 2024 and its quarters, not its months. Rival: an ancestor reaching `k <= n` levels instead of `k < n`. Only this test fails, which is why it exists: the boundary is the whole semantics of N. ### a_toggle_overrides_what_a_group_inherits_either_way_and_toggling_back_clears_it `z` writes `Collapsed` or `Expanded(1)`, or removes the fold where the flip lands on what the group inherits. Rival: never remove. This test and `command::cmd::grid::tests::z_opens_a_closed_hierarchy_one_level_at_a_time_and_closes_it_again` fail; every layout test passes, since a redundant fold draws the same grid. What the rival breaks is the file (a line per toggle, forever) and `z z` being the identity on the view. ### pruning_drops_an_empty_inner_group_and_keeps_its_parent `prune_empty` at depth: an empty quarter goes, header and all, and its year's header stays for the sibling quarter that has data. Rival: `prune_axis_items` keying open runs by level alone. This test and the `NESTED` enumeration fail. ## `model/id.rs` (ADR 007 clause 4 and amendment 1, improvise-cq1q.2, improvise-cq1q.14) Each rival below was injected into the finished module, run against `cargo test -p improvise-core --lib model::id`, and reverted. The reasoning is `context/code/core.md` §20. ### model::id::tests::slugify_case_folds_rather_than_lowercases Rival: `display.to_lowercase()` in place of `unicase`'s full folding. Only this test fails: `ß` lowercases to itself and becomes `_`, so `stra_e` against `strasse`. `slugify_follows_the_amendments_table` cannot see this rival, because each of its displays lowercases and folds alike. ### model::id::tests::slugify_follows_the_amendments_table Every row of the amendment's clause 4 table, plus `-`, `%` and the empty display. Rivals, each of which fails this test: trimming only leading `_` (`East_` stays), trimming `_` and not `-` (`-East-`), not collapsing runs of `_` (`a__b`), not prepending `n` (`2024`), and stripping instead of replacing (`Region(code)` is `regioncode`). The last also fails `slugify_keeps_a_hyphen_between_underscores` and `a_slug_taken_by_another_kind_takes_a_suffix`. ### model::id::tests::slugify_trims_hyphens_and_underscores_until_neither_is_at_an_end Rival: trim leading `_` only, or `_` only at both ends. `_-_East-_-` keeps a hyphen, and `-_-` is not empty. A single `trim_matches` over both characters passes; two sequential single-character trims would not. ### model::id::tests::slugify_trims_before_it_prepends_the_n Rival: no `n` prefix, which fails this test, the table and the property below. `-_7up` is `n7up` only if the trim runs first; prepending first would leave the `-_` in front of the `n`. ### model::id::tests::slugify_output_has_the_id_syntax_and_is_a_fixed_point The property over arbitrary displays: a non-empty slug satisfies `is_id_syntax` and slugifying it returns it. Rivals that fail it (and `generated_ids_are_distinct_well_formed_and_never_reserved`): no `_` collapse, no `n` prefix, keeping a trailing `_` or `-`. ### model::id::tests::id_syntax_agrees_with_an_independent_checker `is_id_syntax` against a hand-written scanner of `^[a-z](_?[a-z0-9-])*$` that does not end in `-`, over `[a-cA-C0-9 _%é-]{0,8}`, so uppercase, space, symbols and non-ASCII are reached. Dropping the alphabet check also fails it. Rivals: allowing `__`, allowing a leading digit and allowing a trailing `-` each fail here and in `claim_refuses_what_a_generated_id_could_not_be`. ### model::id::tests::natural_order_decides_the_zeros_tie_at_its_own_run_not_after_the_rest Rival: compare leading zeros only after every run compares equal in value. `a-7c` then sorts after `a-007b`, and this test fails alone. ### model::id::tests::suffixed_displays_are_distinct_when_literals_already_hold_suffixes Displays are drawn from `A`, `A #2`, `A #3` and `B`, so literal suffixes meet generated ones. Rival: numbering without the skip fails it. ### model::id::tests::claim_refuses_what_a_generated_id_could_not_be Rival: `claim` keeps the old alphabet-only check. `-x`, `x-`, `2024`, `a__b`, `x_` and `-` are admitted, and the test fails. It also pins the accepted ids `east-1`, `item-4`, `a-7` and `q1_-_2024`. ### model::id::tests::claim_admits_each_virtual_id_once Rival: `VIRTUAL_IDS` without `_records` and `_drill`. The test spells the five ids out instead of iterating the constant, because an iteration over the constant passes with the constant shortened (the first version did). ### model::id::tests::natural_order_puts_a_counter_suffix_in_numeric_order Rivals: comparing digit runs as strings (`profit-10` before `profit-2`) fails this test, `natural_order_compares_digit_runs_of_any_length_by_value` and `a_suffix_numbers_equal_displays_in_natural_id_order`; plain `str::cmp` fails those and `natural_order_ties_on_value_go_to_fewer_leading_zeros`. ### model::id::tests::natural_order_ties_on_value_go_to_fewer_leading_zeros Rivals: more zeros first, or no tie-break (`a-7` equal to `a-007`). Both fail this test and the any-length test (`x000` after `x0`). ### model::id::tests::natural_order_puts_the_id_that_runs_out_of_runs_first Rival: the longer id first. Only this test fails. ### model::id::tests::natural_order_is_a_total_order Antisymmetry, equality only for equal strings and transitivity over `[a-c0-3_-]{0,7}`, which includes ids starting with a digit. ### model::id::tests::a_suffix_numbers_equal_displays_in_natural_id_order Rival: ordering the group by `str::cmp` on the ids. `profit-10` is numbered before `profit-2` and this test fails alone. ### model::id::tests::a_suffix_skips_a_number_a_literal_display_already_holds Rival: numbering without the skip. A literal `Profit #2` and a generated `Profit #2` are then both shown, and this test fails alone. A rival that starts numbering at ` #1` also fails it and `a_suffix_is_gapless_when_a_thing_goes_away`. ### model::id::tests::the_suffix_is_the_next_counter_value_not_the_lowest_free_number Rival: the lowest free `-` for each base, which an early draft of ADR 007 proposed and clause 4's shared `gensym` counter replaced. Three tests fail: this one (`"west-1"` against `"west-2"`), `an_empty_slug_takes_its_kind_and_the_shared_counter` (`"item-1"`) and `a_slug_taken_by_another_kind_takes_a_suffix` (`"time_period-1"`). Only interleaving two bases separates one shared counter from per-base numbering, and this test is the one that does it on purpose. ### model::id::tests::a_claimed_id_keeps_its_spelling_and_generation_steps_past_it Rival: `claim` validates the id but does not record it. This test and `claim_refuses_a_duplicate_a_malformed_and_a_reserved_id` fail. The second `East` is issued `east-1`, the file's own id, which is the collision clause 4 forbids: hand-written ids are canonical, and renaming one would corrupt the data section. ### model::id::tests::generated_ids_are_distinct_well_formed_and_never_reserved Rival: `generate` does not record the id it issues, so nothing is retired. Five tests fail, this proptest among them (`reissued`), along with every suffix-order test. The proptest is the one that states the property itself: no id is issued twice in a session, whatever the displays. ### model::id::tests::two_allocators_fed_alike_generate_alike This test builds two allocators independently, so they share no hash state, rather than calling one twice. `seen` is a `HashSet`, but it is only ever queried and never iterated, so its order cannot reach an id (gotcha 18). No rival of the current shape breaks this test. It guards against a future change that picks a candidate by walking `seen`. ### model::id::tests::a_long_claimed_run_of_suffixes_is_walked_without_recursion This is the Codex P2 on PR #318 (review of `a2ddb06e`), reproduced by the adversarial review of `689480bf`. `IdAllocator::suffixed` recursed once per taken candidate, and the call was not in tail position. A new-version file saved from a non-ASCII import claims `item-1` … `item-N`, and the counter restarts at 1 on load, so the next empty-slug generation walks all N. Against the recursive version this test (N = 20,000) aborted the whole test binary: `has overflowed its stack` / `SIGABRT`. It is an abort, not a failed assertion, so no test could catch it. The loop passes, and still advances the counter on every candidate it tries. The second assertion, `category-20002`, pins that: a loop that restarted from a local counter on each call would answer `category-1`. ### model::id::tests::claim_refuses_an_id_that_only_begins_like_a_virtual_one Found by the adversarial review of `689480bf`. With `claim` rewritten to exempt any id that begins with a virtual id (`VIRTUAL_IDS.iter().any(|v| id.starts_with(v))`), all 11 earlier tests passed, because `claim_refuses_a_duplicate_a_malformed_and_a_reserved_id` tries only `_draft`. This test fails on that rival (`left: Ok(())` for `_indexer`). The match is exact by the ADR's own wording: clause 1 says no other id may begin with `_`. ### model::id::tests::a_display_with_leading_spaces_never_slugs_into_the_reserved_space Rival, from the same review: strip the leading `_` before the space step, so that ` Draft` becomes `_draft`. This test fails (`left: "_draft"`), and so does `generated_ids_are_distinct_well_formed_and_never_reserved`. Before this test, the proptest was the only thing that saw the rival. ## Displays beside keys (ADR 007 clause 2, improvise-cq1q.3, first slice) Each renderer test below builds a projection whose display differs from its key. The test-only way to get one is `set_display`, since no production path makes them differ yet. Every test failed before the change, with the key drawn where the display should be: `left: "Region"` against `right: "Sales Area"` in the projection and wasm tests, and a missing "Sales Area" or "Overview" in the TUI buffers. ### ui::project::tests::the_category_panel_shows_a_display_and_keeps_the_key_commands_name It finds the row by `name == "Region"` before asserting on the display. A rival that writes the display into `name` fails the lookup, which covers the half that keeps commands working. `_Measure` is checked to fall back to its own key. ### ui::view_panel::tests::a_view_row_draws_its_display_and_marks_the_active_view_by_its_key Rival: the active marker compares `summary.display == active_view`. This test fails on that rival (no `▶ Overview`), because the active view is a key (`ViewState::active_view`) and its display differs from it. ### render::tests::the_panels_paint_displays_and_keep_keys_for_what_a_command_names The browser half of the same three panels. It also asserts that the view row is still `active` when found by key. ## Displays in the grid frame, page bar, item rows and CSV (ADR 007 clause 2, improvise-cq1q.12) The same test-only way to make a display differ from its key (`set_display`, `set_item_display`, `set_group_display`). Each rival below was injected, the workspace run, and the named tests failed; the revert was re-run green. ### view::layout::tests::a_layout_shows_displays_and_still_addresses_by_keys Rivals: an item's display pushed as its key (`display.push(item_name)`); a page axis's `selected_display` set to `selected`. Both fail on the display asserts, while `cell_key`, `row_group_for` and `row_run_start` stay on keys. ### view::layout::tests::a_folded_group_is_shown_under_the_groups_display_and_keyed_by_its_name A folded row's tuple holds the group name where an item name would be, so the display must come from the group. Not a distinct injection; it covers the arm that the item test cannot reach. ### view::layout::tests::a_records_grid_shows_displays_in_its_headers_and_coordinates Rival: `coordinate_display` returns the item key. Fails here and in `ui::project::tests::a_records_coordinate_shows_its_display_and_edits_its_key`. ### ui::project::tests::the_frame_shows_displays_and_its_key_tuples_still_address_the_cells The rival the ticket names: `axis_frame` writes `tuple: display.clone()`. The test rebuilds every cell's `CellKey` from the frame's key tuples, its `cats` and the page bar, and compares it to `layout.cell_key`, so it fails (and so does the records test, which looks the Region column up by its key tuple). Also fails when a group header's display is its key. ### ui::project::tests::each_view_summary_shows_its_own_display_and_its_categories_displays Rival: every summary takes the ACTIVE view's display. The second view "Plan" has a display unlike both the active view's ("Overview") and its own key, so the fault fails here by name. ### ui::project::tests::an_item_row_shows_its_display_and_keeps_its_key Rival: the item row's display is its key. The unrenamed item "West" must show its key, and `_Measure`'s "Revenue" shows "Sales". ### ui::project::tests::the_page_bar_carries_displays_beside_the_keys_a_page_command_names Rival: `selected_display` is the key. Fails here, in the layout test, in the TUI grid test and in the CSV test. ### render::tests::a_header_draws_displays_and_blanks_a_repeat_by_key Rival: suppression compares display tuples. West shares East's DISPLAY ("Same") and must still draw, so comparing displays blanks it. ### render::tests::a_header_is_as_wide_as_what_it_shows Rival: `header_widths` measures the key tuples. Fails (key "E" against display "Eastern Seaboard"), as does the TUI grid test. ### render::tests::a_frame_with_ragged_displays_is_refused Asserts the control parses, then that a `cat_displays` shorter than `cats` and a display tuple shorter than its key tuple are each refused with their own message. ### resolve::tests::hiding_a_displayed_row_item_names_its_key Rival: the hide resolver reads the first DISPLAY of the row frame. It composes `HideItem { item: "Eastern Seaboard" }` and the test fails on `"East"`. ### resolve::tests::picking_a_displayed_page_item_still_names_its_key Picks index 1 of a page axis whose displays are not its keys and requires the command to name `Q2`. The index is the only thing that travels from the menu. ### render::tests::the_grid_page_bar_and_item_rows_paint_displays The browser half. Rival: the cycle's `items` relays the keys, which fails here. ### ui::grid::tests::the_frame_and_page_bar_draw_displays_not_keys The TUI half. Rival: the group header draws `group.name`; fails on the missing "▼ Bay". The key assertions use strings that appear only as keys. ### ui::category_panel::tests::an_item_row_draws_its_display_not_its_key The TUI item row. Not separately injected: its draw path is the one line the `CatTreeEntry::Item` pattern changed. ### persistence::export::tests::export_csv_writes_displays_in_every_header_and_label Rival: the column header uses `col_label` (the key). The exact output pins the corner cell `Kind (Who=Alicia)`, the column header and the row label. ### view::layout::tests::a_page_axis_with_ragged_displays_is_refused From Copilot's finding on #336: `item_displays` is a parallel list on a wire type that derives `Deserialize`. The control parses a well-formed axis first; then a shorter list and an absent one beside `items: Some(..)` are each refused with the "item displays" message. Without the `try_from` the same JSON parses. ### ui::viewport::tests::a_column_is_as_wide_as_the_display_its_header_draws From the Opus review of #336: `header_level_seed` could measure keys and the suite stayed green. The fixture's column display ("Januaryyyyy", 11) is far longer than its key ("Jan") and the cell, so the 5-column minimum cannot hide it. Rival: the seed reads `tuple` instead of `display`; the width is 5, not 12, and this test fails. ### ui::viewport::tests::the_row_header_is_as_wide_as_the_display_its_rows_draw Rival: `compute_row_header_width` reads `tuple`. That width drives `visible_cols` on the keypress path (the improvise-gtn7.1 shape: a scroll measured elsewhere than it is drawn), so a key measurement lets the cursor scroll to an undrawn column. The display "Eastern Seaboard" against the key "E" gives 17, not the 4 minimum. ### render::tests::a_column_group_header_is_painted_by_its_display Rival: `col_groups` labels a run with `g.name`. The browser test for group displays covered row groups only; this one folds a column frame whose group "Q1" is shown as "First quarter" and fails on the key. ### draw::content_tests::the_grid_title_names_the_views_display_not_its_key improvise-cq1q.13, TUI half. The view is displayed "Quarterly overview of regional sales" under the key "Default", and the whole frame is drawn through `draw`. Rival: `draw_content` passes `app.view_state.active_view` to the widget; the title reads `View: Default` and this test fails on both assertions. ### render::tests::the_title_names_the_active_views_display_not_its_key The browser half. Rival: `render_model` takes `view_name` from `state.view.active_view`; the test fails. The second half is the control: a view the list does not hold falls back to its key rather than to an empty title. ### resolve::display_tests::a_panel_status_names_what_a_person_sees_not_the_keys The browser resolver's panel statuses: axis cycle, filter, delete category, delete item. Displays are 36 to 38 characters against keys of 4 to 6. Rivals, each run: the cycle status passes the key; the filter composes `SetPageSelection` with the item's display; the item removal names the display. Each fails here, the last two on the `Region/North` and `RemoveItem` assertions that pin the lookup to keys. ### resolve::display_tests::a_tile_status_names_the_display Rival: the tile route passes `tile.category`; fails on the key. ### resolve::display_tests::the_item_prompt_names_the_display_and_the_mode_keeps_the_key Rival: the mode is built with the display as its category (`item_add_shown(shown, ..)`); the prompt reads right and the commit would add to a category that does not exist. The test fails on `category == "Region"`. ### resolve::display_tests::a_refused_cell_names_the_category_it_shows Rival: the non-item refusal gets the category key. The stray coordinate stays a key because it names no item and has no display. ### command::cmd::panel::tests::a_panel_status_names_what_a_person_sees_not_the_keys The host half of the panel statuses and the item-add prompt. Rivals, each run: the axis status passes `cat_name` (fails); the filter status passes the item key (fails); the item-add mode is built from the display (fails on the `category: "Type"` assertion). Review follow-up: the same test now also pins the operands. Rivals: the host `RemoveCategory` and `SetPageSelection` given the display fail on the `RemoveCategory("Type")` and `item: "Food"` assertions. The client twin fails the same way on `RemoveCategory`. ### command::cmd::tile::tests::a_tile_status_names_the_display_not_the_key Rival: the status formats `name`. It fails on both the present display and the absent key. ### ui::app::non_item_key_tests::a_non_item_refusal_names_the_categorys_display Rival, run once per site: the refusal in `EditOrDrill` (the `i` key) and the one in `CmdContext::value_cell_key` (the `p` key) each pass the key. Each fails here. ### ui::app::non_item_key_tests::a_commit_on_a_non_item_cell_names_the_categorys_display The third site, `uneditable` in `cmd/core.rs`, which only a commit reaches once an editor is open (`:enter-mode editing`). Rival: it passes the key; the two tests above stay green and this one fails. ### ui::app::non_item_key_tests::a_refused_wire_write_names_the_category_it_shows The wire's own refusal in `Grids::unwritable`. Rival: the category key; fails. The formula arm (once a third copy of the derived wording, now `derived_refusal`) is pinned to its wording so the consolidation cannot change what it says. ### resolve::display_tests::the_declines_an_advance_and_a_commit_reach_name_the_category_it_shows From the Opus review of #339: the browser's `declined_at` non-item arm could pass the key with the whole suite green. It is reached by `reopened_at` (an advance), `refused_here` (a commit) and `settle_declined_editor`. Rival: the arm passes `category`; the test fails on all three assertions, and the display is 36 characters against the key "Region". ### model::source::tests::a_summary_reduces_as_the_values_did improvise-qddo. The reduction moved from `aggregate_cell_values` to `Summary::reduce`; the test keeps the old function as `reference` and compares all 585 value sequences of up to three values from an eight-value pool (NaN, both zeros, infinity, two texts, an error) on all five functions. Rivals, each failing it by name: unanimity ignored (`Others::One` kept for any second value), a number beside a unanimous text read as unanimous, and `<` for the order of the zeros (the sign of a zero extreme is lost). The first two also fail `aggregating_differing_text_values_is_an_explicit_error` and `aggregating_text_among_numbers_is_an_explicit_error`; the zero rival also fails `eager_math_tests::min_and_max_of_both_zeros_do_not_depend_on_their_order`. ### model::source::tests::a_merge_of_summaries_is_the_summary_of_the_union Review of #356 (Opus): the pool gained `1e16`, `1.0`, `-1e16`, `0.1` and `0.2`. A merge rounding each part's sum before pooling, keeping the zero sign, changed 514 production answers while the workspace passed; that rival now fails this test, `numbers_pool_what_the_formula_aggregate_pools` (a NaN-free split now, compared as bits), `summaries_built_from_parts_merge_as_the_union_does` and the fixture test below, by name. A read of several parts is a merge. The test merges every ordered pair of sequences of up to two values and compares with the reference of the concatenation. Rivals: the least not merged, the greatest merged with the other side's least, the zeros unordered. Each fails it. ### model::source::tests::numbers_pool_what_the_formula_aggregate_pools `eval_agg` pools `Numbers` now, not a concatenated `Vec`. Every split of a five-number list (a NaN and both zeros among them) pooled by merge must equal the whole, by bits for all five functions and for the total. Rivals: the least not merged, the greatest seeded from the other side's least. Both fail it. ### model::source::tests::the_store_and_a_scanned_table_answer_every_select_alike `DataStore` and `TableSource` (a scan, sharing only `Summary`) are asked every select over the fixture, at both widths. Rivals, each failing it: the store ignores `exact` for a select with no span, and the store ignores `exact` for a spanned one. The first also fails `an_exact_select_with_no_span_names_one_cell`. ### model::source::tests::the_store_and_a_scanned_table_list_the_same_cells `keys` and `cells` under every base of the fixture, and `value_at` for every row and an absent key. Rival: `DataStore::keys` ignores its partial. It fails here and in seven evaluator tests that read the member records of an aggregate over an expression (`route_then_aggregate_tests::a_routing_formula_at_a_record_is_the_rule_itself` among them). ### model::source::tests::an_exact_select_with_no_span_names_one_cell A select with `exact` and no span is the cell, not the cells containing the pairs. The store's `spanned_values` is never called for it (the evaluator reads an exact key through `value_at`), so this is the contract a second source implements. Rival: `exact` ignored when there is no span. ### model::over_tests::a_model_over_a_source_evaluates_as_the_model_that_owns_the_cells improvise-rnak. A model with its store emptied and `over` a `TableSource` of the same rows is asked every read the evaluator makes (exact and over-hidden values, covered by a view that hides East, spanned over none, one and two categories) at every sub-key of every stored key, a group named in place of a member, and every formula target; the answers must agree to the bit. The fixture has a ref, an aggregate of a ref, an aggregate of an expression, a filtered formula, a cycle and a formula over formulas, and 2,000+ answers are compared. Each rival replaces one read with the owned store, and fails it by name: the member records of an aggregate over an expression, the pooled formula aggregate's raw read, the fold of a group, the exact read, and the stored cell a filtered formula falls back to (a `Bonus` cell stored where its `WHERE` does not hold; before that cell was added to the fixture this rival passed). ### model::over_tests::the_formula_cells_of_a_model_over_a_source_are_the_owned_models `recompute_formulas` over a source under three None sets: the formula cache must equal the owned model's at every key, and at least 40 cells are present across the three (a cache that is empty on both sides would pass the comparison). Rival: the stems read from the owned store, which finds no stem and fills no cell; it fails here and nowhere else. ### model::over_tests::a_model_over_a_source_does_not_read_its_own_store A cell is set in the over-model's own store (the only cell it holds) with a value no fold could mistake. The exact read and the fold of its measure must be the source's. Rivals: a fold or an exact read of the owned store fail it. ### model::over_tests::the_records_a_drill_freezes_are_read_from_the_source `drill_snapshot` (records mode's rows) over the empty store and a source. Rival: the unconstrained branch reads the owned store; it fails here. ### model::over_tests::a_model_over_a_source_refuses_a_write `set_cell` on a model over a source panics in a debug build, naming the reason. Without the assert the write goes to the owned store and nothing reads it. ### a_summary_built_from_an_engines_parts_reduces_as_one_built_from_the_values Review of #356 (Codex): an engine outside core could not build a `Summary` without replaying rows. The parts are computed in the test without `Summary::of`, for every sequence of up to three values of the pool. No rival of the first version failed this test alone (its filter was untested); see below. ### summaries_built_from_parts_merge_as_the_union_does A summary built from parts, merged with one built from values and with one built from parts, equals the reference of the union, for every pair of sequences of up to two values. This is where a bad extreme would matter, since merge reads the extremes. ### parts_no_sequence_of_values_could_produce_are_refused Review of #356 (Opus, Codex): `from_parts` repaired some impossible states and accepted others (`One(Number)`, least above greatest). It refuses all of them with a `BadParts` naming the reason. The earlier entry's claim that a NaN-extreme rival failed `parts_that_contradict_themselves_...` was false: that test passed with the filter removed. Rivals now, each failing this test alone: NaN extreme allowed, least above greatest allowed, extremes with no numbers allowed, a number accepted among the others. ### the_exact_sum_of_a_pool_of_cancelling_values_is_not_the_sum_of_rounded_parts An exactness test needs inputs whose float sum is inexact and no NaN. `1e16 + 1.0 - 1e16` rounded part by part is `0.0`; pooled exactly it is `1.0`. This pins the fixture rather than the code. ### keys_of_a_store_are_its_cells_keys_without_the_values Review of #356 (Codex): `DataStore::keys` went through `cells` and cloned every value. It reads keys only now; the test pins the keys and their order against `cells`. No clone-count test: a value clone is not observable from outside, so the equality is all that can be asserted. ### a_model_over_a_source_refuses_a_clear Review of #357: the `clear_cell` guard had no test. The refusal is an `assert!`, so the test holds in a release build too (`cargo test --release -p improvise-core --lib over_tests` was run: the earlier `debug_assert!` failed all four refusal tests there, `a_model_over_a_source_refuses_a_write` among them, and was the bug). Rival: the guard removed from `clear_cell` fails this test only. ### a_model_over_a_source_refuses_to_remove_a_category `remove_category` swept the owned store with no guard. Rival: the guard removed. ### a_model_over_a_source_refuses_to_remove_an_item The same for `remove_item`. ### putting_a_populated_model_over_a_source_drops_its_own_cells `Model::over` empties the owned store, so `get_cell` on an over-model is `None` for a cell the source holds, and the evaluator still reads the source's value. ### records_mode_reads_the_records_from_a_source Review of #357: a records-mode `GridLayout` over a source. `group_records` reading the owned store left all 563 tests green; with this test it fails (the grid has no rows). ### putting_a_model_with_formula_cells_over_a_source_drops_what_those_cells_computed Review of #357 (Codex): `Model::over` emptied the store and kept `formula_cache`, which was computed from those cells; reads consult the cache first, so old formula values survived until a recompute. The model has a computed Profit, the source holds a different revenue, and the cell must be gone after `over`. Rival: the `formula_cache.clear()` removed fails this test alone (before the fix it failed here first). `measure_agg` and the categories are the definition and stay; nothing else is derived from the cells.