The Status now says the tag is the complete list of proposed rules, and its table gains the regression test's step 3. Clause 12's test keeps the accepted "commit the unedited text" with the amendment's edit-a-non-name step tagged; the outside-_Measure real-item rule, the formula-collision example and the East/East_ collision sentences are tagged; the "exactly one id" sentence in clause 2 is restored; "other entities" loses the added "virtual"; the title drops "and groups". core.md section 20 and the id.rs test evidence cite ADR clauses instead of answer numbers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
108 KiB
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<String> 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 Cmds,
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(<expr>) 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 DataItems — expand_category
emits the GroupHeader and then continues 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 <cat> <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 GroupEnds 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::<u8>().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::<u8>() 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::atreturningself.viewunconditionally (the override never applies) fails withleft: "0.67" right: "0.6667".FormatPlan::atreturning the first entry in the plan for any key (the override leaks to every cell) fails withleft: "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<category, HashMap<item, _>> 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 <n> 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
deriveon thef64— the state at HEAD — fails this test atleft: "{\"Number\":null}"and also fails the io-side agreement test. - Drifting the shared token (
-inf→-infinity) fails this test, the agreement test, andgenerated_file_roundtrips, because the writer and the reader stop agreeing. - Re-copying the tokens into
persistence/format.rsso 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
continued 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 GroupEnds
(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' MAXes 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_formulamakes 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, fromopen_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:
RegionoverMonth;- three levels,
RegionoverMonthoverKind; Month = [Jan in Q1, Mar, Feb in Q1], a non-contiguous group, so one parent holds twoQ1runs (improvise-h0iv);- a middle level with no groups at all (
KindbetweenRegionandMonth); - two outer groups side by side (
NorthinUpper,SouthinLower).
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 Workbooks 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
GroupEndfails 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, atfixture 2, mask 0b101, collapsed []: run identity at level 1, row 1. PruningMarmust leave North's twoQ1runs 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
GroupEnds 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/Janis inMonth/Q1, the innermost;North/Maris inRegion/Upper, becauseQ1ended andUpperdid not;South/Maris 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 anExactfold 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_findsfail. 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_inpassingAggregation::Exactinstead 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_collapsesandan_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_rowswith a fresh evaluator per cell:left: 45, right: 9.one_grid_build_evaluates_each_fold_formula_once_for_cells_and_totalswith 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, improvise-cq1q.2)
These tests were written before slugify and IdAllocator had bodies. With
todo!() bodies all eleven failed. Each rival below was then 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: left: "strae", right: "strasse". ß lowercases to itself
and step 3 strips it. slugify_follows_the_adrs_examples cannot see this rival,
because each of its displays lowercases and folds alike.
model::id::tests::slugify_collapses_a_run_of_spaces_to_one_underscore
Rival: every space becomes _, with runs not collapsed. Only this test fails:
"time___period". No other fixture has two spaces in a row.
model::id::tests::slugify_keeps_an_underscore_the_display_already_had
Rival: .replace("__", "_") after the space step, which reads the accepted
clause 4 as "no doubled underscores". Only this test fails: left: "a_b". The rule
collapses runs of spaces. An underscore that was in the display is kept.
model::id::tests::the_suffix_is_the_next_counter_value_not_the_lowest_free_number
Rival: the lowest free <base>-<n> 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.