Files
Edward Langley f187fa57fe Merge gh/main; fix the whitespace run in the refusal message
Conflict was `context/test-evidence/gp.md` only — this PR and #146
(improvise-ipqu) appended at the same point. Both entries kept, verified
line-by-line against both parents, because --check-evidence cannot see a
dropped entry: it checks that every heading names a running test, and neither
of this PR's tests cites the file, so losing one would pass the gate.

The format string was broken across source lines with no `\` continuation, so
eighteen spaces were baked into what a user reads on --predict against a
corrupt model. Found by adversarial review, which printed the message rather
than reading the source.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
2026-09-22 19:39:07 -07:00

1263 lines
68 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Test evidence: `gp-tree` and `improvise-gp`
What each test's bug was, what fault was injected to prove the test can fail,
and why the obvious weaker version of it proved nothing. Keyed by exact test
name — a rename orphans the entry, which
`scripts/check-comment-runs.py --check-evidence` catches.
The architectural arguments these tests defend are in `context/code/gp.md`.
---
## `gp-tree/src/lib.rs`
### splitmix64_matches_the_reference_vectors
The vendored RNG's constants had nothing pinning them. Changing the gamma from
`0x9E3779B97F4A7C15` to `…7C17` left **every other test in the workspace
green**, because they only ever assert that two runs at one seed agree.
The crate's promise is stronger: a fit's seed is printed to the user and saved
in `data::Model`, so a *later* run at an old seed must reproduce the old fit.
These are therefore the published SplitMix64 vectors for seed 0, not a golden
capture of this code — which is what makes them evidence rather than a
tautology.
### splitmix64_matches_the_reference_vectors_from_a_nonzero_seed
Seed 0 alone cannot catch a change to how the seed enters the state
(`state = seed` versus `state = 0`), because both give the same stream at 0.
Also asserts `seed()` still answers the seed after the generator has advanced.
---
## `gp-tree/src/fit.rs`
### mse_averages_over_exactly_the_rows_it_was_given
`mse` used to take parallel `x`/`y` slices, sum over `zip(x, y)` — which stops
at the shorter — and divide by `x.len()`. A caller that got the two out of step
was handed a mean that was quietly **too small** rather than an error: the
improvise-n3wt hazard, a count supplied alongside the data rather than taken
from it.
The remedy was not a length check but a type: one slice of `Row`, so the shorter
slice cannot be built (improvise-dkqn). This test is what is left to say about
it — numerator and denominator count the same rows, for any prefix.
### scoring_no_rows_is_zero_error_rather_than_nan
Every mean in the module reached `0.0 / 0.0` on an empty slice and handed back a
NaN a caller could not tell from a score: it propagates through
`FitConfig::cost` and sorts unpredictably against real costs. One answer, in
`mean_over`, covers `mse`, `misclass_rate` and `standardize`, and the test
asserts all three plus `std == 1e-6` (a degenerate spread still never divides by
zero).
### asking_for_no_folds_yields_one_rather_than_dividing_by_zero
`k` reaches `folds` from a CLI flag, so zero is a value a caller can supply
rather than an impossible state. `out[pos % k]` divided by it and panicked — a
public library function taking its host down with it.
### cross_validating_without_a_held_out_fold_pools_nothing
The sharper half of the same parameter (improvise-ry89). A cross-validated
prediction is one made by a tree that never saw the row it scores, so `k < 2`
asks for something that cannot exist: the single fold holds every index, the
training complement is empty, and the tree is fit and standardized on nothing
before predicting every row.
**This used to announce itself** — `standardize(&[])` gave a NaN mean and every
pooled prediction came back NaN. Once `mean_over` answered the empty row set
with `0.0`, the same call handed back near-zero numbers indistinguishable from a
model's output, and an R² computed off them is a number rather than a NaN. So
fixing one bug hid another, and the count is the contract: no held-out row means
no pair. The test also asserts the generator is **untouched**, because declining
before the shuffle is what keeps a caller's next draw the one it would have made.
### the_training_split_is_what_the_contains_scan_gave
The O(n²) fix (improvise-t3ux) replaced `test.contains(i)` per row with a bool
mask. It is only safe because the mask yields the *same indices in the same
order* — the training rows are what every downstream draw from the generator
depends on. Pinned directly, at every fold count, rather than inferred from a
fit that happened to agree.
### a_fit_improves_on_the_tree_it_started_from
Without it, a `fit` that returned its seed tree unchanged would satisfy both
`the_same_seed_and_config_fit_the_same_tree` and
`a_different_seed_fits_a_different_tree`, because the initial `random_tree`
already differs across seeds.
### fit_detailed_starts_from_the_best_of_generation_zero
What `start` has to mean for the question it exists to answer: the best of
generation 0, not the seed ancestor it grew from (improvise-ut7p). The two
differ whenever a mutated member of the initial population beats member 0, and
only the former makes "the search improved on where it started" a claim about
**breeding**. Before this, `improvise-gp` kept a second copy of the whole GA
loop just to reach that starting point.
### a_tiny_lone_importance_is_still_the_whole_of_it
Copilot on PR #25. `fit_csv` divided by `total.max(1e-9)`, which is right for
the only input it ever had and wrong as a contract: one feature carrying an
importance of `1e-10` carries all of what the model knows and was reported at
**10%**. Importances this small are ordinary — they are error differences over a
*standardized* target. `the_shares_sum_to_a_hundred_at_any_scale` is the general
form across four orders of magnitude.
### the_cli_refuses_an_overflowing_target_rather_than_aborting
Found by ChatGPT Codex on PR #111, as a P1, and correct. improvise-pv3g's claim
is that a finite input no longer aborts the process; the two *library* paths
honoured the refusal and `examples/fit_csv.rs` turned it straight back into a
panic with `expect`, at `fit_csv.rs:399`. A user with a 1e20 column still got a
panic — at a different site than before, but still a panic, from the entrypoint
a person actually runs against a CSV.
Demonstrated before fixing: a temporary test drove `run_fit` over an 8-row CSV
at 1e20/4e20 and aborted at exactly that line. `run_fit` now returns
`Result<(), String>` and `main` prints the refusal and exits 1 — gotcha 37(a) is
the reason the example is the right place for that translation, since the
library returns data and the CLI owns every message.
The split that makes it testable is `fit_and_report`, which takes the header and
rows already in hand; `run_fit` is the CSV-reading shell over it. The example
carries `test = true` in `crates/gp-tree/Cargo.toml` so its `#[cfg(test)]`
module runs under `cargo test --workspace` (examples default to `test = false`).
Rivals injected: restoring the panic fails this test and leaves its siblings
green; refusing every map fails the just-under test and leaves this one green.
### the_cli_still_fits_a_target_just_under_the_overflow
The half that keeps the refusal from being satisfied by refusing everything. It
sits at `1e9` rather than `1e18` for a reason worth reading — see below.
### the_cli_does_not_abort_when_an_importance_share_overflows
**The second abort on the same path, and it was found because the first draft of
`the_cli_still_fits_a_target_just_under_the_overflow` used `1e18` and blew up
somewhere else entirely.** At that scale `standardize` succeeds, the fit lands,
and `report_importance` aborts with a **capacity overflow** in
`"#".repeat((share / 5.0).round() as usize)`.
The cause is upstream and is filed as **improvise-k844**:
`importance_ranking` computes `100.0 * imp[j] / total`, and the multiplication
happens first, so an importance above `f32::MAX / 100` = 3.4e36 overflows to
`inf` before the division that would have returned 100.0. Measured here:
`imp = [3.7499995e36]`, `total = 3.7499995e36`, `share = inf`. A saturating
`as usize` on that is an enormous allocation, which is an abort and not a
diagnostic — the exact thing improvise-pv3g exists to remove, one function
further along.
PR #111 clamped the bar width and filed the arithmetic, rather than reordering
the expression, because `misclass_rate_and_importance_are_bit_for_bit_what_they_were`
pins importance as f32 bit patterns and that PR's headline evidence is that no
scoring bit pattern moved. Injecting the clamp's removal fails this test and
only this test. What survives to k844: the CLI still *prints* `inf%`.
### standardize_refuses_a_spread_that_overflows_f32
improvise-pv3g, a P1 that crashed the TUI. `fit-measure` panicked at
`number_text`'s `assert!(v.is_finite())` on an ordinary document whose target
values were large but **finite** — 8 records with a `Score` of `1e20`/`4e20`.
Every input was finite, and `improvise-gp::fit_measure` checked exactly that;
the overflow was in a **derived** quantity. `standardize` sums `(t - mean)²` in
`f32`, and `8 × (1.5e20)² = 1.8e40` is past `f32::MAX = 3.4e38`, so `std` came
back `inf`. Every normalized target then became `±0`, the champion's leaves sat
near zero, and `denormalize` computed `0.0 * inf + mean = NaN`.
Reachability scales with the record count — the condition is
`sum((t-mean)²) > f32::MAX`, so the crashing spread is about `sqrt(3.4e38 / k)`:
~6e18 at 8 records, ~1.3e17 at the 20,000-record ceiling.
The fix is a type: `Scaling` has no public field and its only constructor
refuses a non-finite part, so `standardize` returns `Option<(rows, Scaling)>`
and `denormalize` cannot be handed a poisoned map at all
(`doctrine(an_invariant_is_made_unrepresentable_not_validated)`, the answer
`Row<T>` and `prune_axis_items` got). The arithmetic is untouched:
`standardize_is_bit_for_bit_what_it_was` passes with every bit pattern
unchanged.
**Measured, and it corrects the ticket.** The ticket's sibling path — "`sum(t)`
overflowing gives `mean = inf`, and `NaN.max(1e-6)` returns `1e-6`, so `std` is
FINITE while `mean` is infinite" — does not occur from finite inputs. A finite
target with an infinite mean gives `(t - inf)² = +inf`, so the variance is
`+inf` and never `NaN`, and `inf.max(1e-6)` is `inf`: `std` is non-finite too.
`NaN.max(1e-6)` laundering needs an **infinite target**, which the pre-existing
input guard already refuses. Injecting `Scaling::new` guarding only `std`
(dropping the `mean` half) fails **no** `standardize` test — only
`a_scaling_cannot_carry_a_non_finite_part`, which exercises the constructor
directly. The `mean` half is therefore a contract of the *type*, not a reachable
`standardize` path, and is kept on those terms.
Rivals injected: guarding only the `mean` (fails four tests, including the
end-to-end one); refusing every map (fails the just-under tests in both crates,
which is what keeps the guard from being "refuse everything large").
### standardize_refuses_a_target_whose_sum_overflows_f32
The other arithmetic, with a spread small enough to square: eight rows all at
`1e38` sum to `8e38`, so `mean = inf` before any square is taken and every
normalized target is `(1e38 - inf)/inf = NaN`. Separate from the spread case
because the two overflow at different magnitudes — this one needs the *sum* past
`f32::MAX`, which the spread case reaches far earlier for a varying target.
### a_scaling_cannot_carry_a_non_finite_part
The floor exists for a degenerate spread and must not launder a poisoned one:
`NaN.max(1e-6)` is `1e-6` in Rust, which is how a poisoned variance used to pass
a finiteness check and surface as a panic somewhere else entirely. So the floor
lives **inside** the constructor, after the finiteness answer, and the test
asserts both halves: `±inf`/`NaN` in either position is `None`, and
`Scaling::new(0.0, 0.0)` still floors to `1e-6` rather than being refused.
### a_fold_whose_target_overflows_contributes_no_pairs
The silent half of improvise-pv3g, and the reason a fix at `number_text` would
have been the wrong one. `cv_regress` standardizes each training fold and pools
`eval(tree) * std + mean`; with a poisoned map that produced quality numbers —
an R² a user reads and believes — with **no panic to announce it**. A fold that
cannot be standardized now pools nothing, which is the same answer
`cross_validating_without_a_held_out_fold_pools_nothing` gives: no evidence
reads as no evidence.
There is no fault to inject on the pooling side any more, and that is the point:
a poisoned `Scaling` is not constructible, so the rival implementation cannot be
written.
### denormalize_rescales_the_leaves_and_leaves_the_splits_alone
The negative arm is the load-bearing one: `standardize` rescales the **target**,
so a threshold is already in its feature's own units and re-scaling it with the
leaves would be the bug.
`a_tree_fitted_on_standardized_rows_predicts_original_units_after_denormalize`
pins the arithmetic against a hand-built tree rather than a search, so it does
not depend on the GA.
---
## `gp-tree/src/data.rs`
### an_entirely_missing_column_is_skipped_and_reported_as_data
`preprocess` used to say so with an `eprintln!`, which was correct while this
code was `examples/fit_csv.rs`. A library does not own its caller's stderr: a
TUI on an alternate screen, a wasm build, or a caller wanting the notice in its
own status line could none of them suppress, redirect or translate it. The
notice now comes back as data.
### seed_zero_is_distinguishable_from_no_seed
`0` is a legal seed, so "unknown" and "fitted at 0" have to be two different
values — anything reporting the seed of a pre-seed model would otherwise state a
seed that never ran. That is why `Model::seed` is an `Option` and not a
sentinel.
### a_saved_model_round_trips_with_its_whole_ga_config
The seed alone is necessary and **not sufficient**: the same seed under a
different `FitConfig` searches differently and lands somewhere else, so a model
carrying only a seed looks reproducible while being reproducible only by whoever
still remembers the flags.
The fixture `a_fit_config()` differs from `FitConfig::default()` in **every**
field, and the test asserts that explicitly. A round trip that silently
substituted the default would otherwise pass against a default-constructed
value — the failure mode this test exists for.
### a_model_saved_before_configs_still_loads
A backward-compatibility test aimed at the **absent field**, not at the
attribute. `#[serde(default)]` on an `Option` is documentation rather than
mechanism — serde defaults an `Option` anyway, so a test that merely deletes the
attribute fails nothing and is not discriminating. This one removes `config`
from the serialized JSON and asserts it loads as `None` (unknown), not as
`Some(FitConfig::default())`. `a_model_saved_before_seeds_still_loads` is the
same shape for `seed`.
---
## `gp-tree/tests/scoring_known_answers.rs`
Whole-file note: these are `f32` **bit patterns**, not approximate values,
because a change that merely moves an accumulation around silently re-points
every seed already saved at a different tree. `assert!((a - b).abs() < 1e-6)`
cannot see that, and neither can the CLI's output, rounded to three or four
decimals. The numbers were captured from the parallel-`x`/`y` code *before*
improvise-dkqn reshaped the signatures and improvise-t3ux replaced the
`contains` scan, and are unchanged by both. If you are changing the search on
purpose these numbers change with it — but then every seed recorded by an older
`--save` names a different tree, and that is the decision this file forces you
to make out loud.
### standardize_is_bit_for_bit_what_it_was
**A witness that could not see the bit.** The first and last rows plus
`mean`/`std` were the whole witness, and four numbers cannot separate two ways
of computing the same quantity: injecting `r.target / std - mean / std` for
`(r.target - mean) / std` — algebraically identical, a different rounding — left
this test **and both cross-validation vectors** green (improvise-ry89). All
thirty-seven values do separate them: that reassociation moves 21 of the rows,
and none of them were rows 0, 36 or 19.
Note what is *not* evidence here. Reversing the mean's accumulation order also
left every vector green, but on this fixture the two orders sum to the same
`f32` — so that injection changed no behaviour to catch. **A perturbation has to
move a bit before a bit-level witness can be blamed for missing it.**
### cv_regress_pools_the_same_predictions_and_leaves_the_rng_where_it_was
The `rng_after` witness is the part that pins the *order* of the generator's
calls: the next `u64` after a cross-validation depends on exactly how many draws
that run made, so a reshaped loop that drew differently would move it even if
every pooled prediction happened to agree. 37 rows is a prime, so no fold count
divides it evenly and the last fold is short — where an off-by-one in the
train/test split shows.
### cv_classify_pools_the_same_codes_and_leaves_the_rng_where_it_was
The classification half of the same guard. The mask replaced `contains` in both
loops, and one arm passing says nothing about the other
(`doctrine(one_arm_of_a_symmetric_rule_is_evidence_about_that_arm_only)`).
---
## `improvise-gp/src/lib.rs`
### improvise_evaluation_matches_gp_tree_eval
The acceptance property: over random trees and random feature vectors,
improvise's evaluation of the emitted formula matches `gp_tree::eval`
**exactly** — widening f32 to f64 is lossless and the thresholds are exact, so
there is no tolerance to allow.
#### The `improvise_eval` helper, and the shape to remember
This test and `a_value_sitting_on_the_threshold_falls_the_same_side` both run
through the `improvise_eval` helper, and its scope is what was once read as
covering more than it does. **It fabricates a `_Measure` item per feature
name**, so it asserts the emitter and `gp_tree::eval` agree **against a model
that could not exist**. It says `Ok` to `outlook=rainy` because it just made an
item called that. A helper that builds the world the code wants is not evidence
that the world exists.
So the awkward names above are a *quoting* fixture and nothing more:
`outlook=rainy` is there because it is awkward to spell, not because a document
could hold it — it could not, and `fitted_formula` refuses it. The helper is
still the right tool for the arithmetic-and-quoting claims; the property it
could not state is asserted separately, against a real document, in
`evaluability_in_a_real_document`.
### an_awkward_target_name_is_quoted_and_round_trips
This test used to assert `parsed.target == "|Total Revenue|"` and call the pipes
correct. They are not: the target names a `_Measure` item, so a target carrying
its delimiters writes the fit into an item literally named `|Total Revenue|`,
which is not the item the user has. What the emitter owes its caller is a
*semantic* round trip — the name goes in, the same name comes back out.
`an_awkward_target_names_the_users_measure_item` is the consequence that makes
it a data bug rather than a cosmetic one.
### an_infinite_threshold_is_refused_rather_than_emitted_as_inf
`number_text(f32::INFINITY)` emitted `"inf"`, which `"inf".parse::<f64>()`
accepts — so the round-trip loop returned it and a `formula_text` caller got
text the grammar rejects, with nothing on the way saying so. `fitted_formula`
would surface it as a parse error; a bare `formula_text` caller would write it
into a file.
### a_nan_threshold_is_refused_by_name
NaN fell through the loop to `unreachable!`, so the panic blamed the decimal
expansion instead of the caller's value.
### a_threshold_is_the_shortest_decimal_that_reads_back_the_same
The doc used to overclaim the emitted text as the f32's full decimal expansion.
It is the *shortest* decimal that reads back as the same `f64` — the weaker
claim, and the one the routing argument actually needs: equal text means equal
`f64` means the same branch.
### a_value_sitting_on_the_threshold_falls_the_same_side
The boundary case the whole `number_text` design exists for. `0.7f32` is really
0.699999988…; the shortest text that reads back as that **f32** is `"0.7"`,
which parses as an `f64` strictly *greater* than it. A row whose feature value
equals the threshold routes `hi` under `eval` and would route `lo` through a
formula carrying the rounded threshold. The test asserts the trap is live —
`"0.7".parse::<f64>() > thresh as f64` — before relying on it.
### a_split_on_a_one_hot_feature_is_spelled_as_a_coordinate
improvise-8xhy's defect, and improvise-4zl7's answer to it. `preprocess`
one-hots a categorical column into features named `Region=North`, an encoding
artefact no document has an item for, so `find_item_category` returned `None`
and every cell of the fitted measure came back as `Error("ref:Region=North")` —
silently, with the emitter having said nothing. 8xhy shipped a refusal; 4zl7
replaced the refusal with a spelling, `Region IS North`.
The assertion is the emitted **text**, exactly, and not merely that the call is
`Ok`: the whole failure mode here was a well-formed formula meaning the wrong
thing, so "it parsed" is the assertion that missed it the first time. The
fault to inject is `Spelling::Coordinate` rendering as `quote_name(name)` —
byte-identical for every measure, and wrong for exactly this family.
### a_coordinate_naming_no_category_of_this_document_is_refused
The negative arm of the same rule, and it is not redundant with the artefact
refusal: a `Coordinate` is *spellable* and may still be unresolvable. Without
it, `resolves_in`'s `Coordinate` arm could return `true` unconditionally and
every test above would still pass.
### a_measure_that_merely_looks_like_a_one_hot_is_accepted
The negative arm, and why the check asks the **document** rather than refusing
names that look like encoding artefacts: a measure literally named
`Region=North` is a legal document. The two are indistinguishable as strings.
`the_missing_flag_and_the_date_sub_features_are_refused_too` covers the other
two artefact families with nothing enumerating them — the check asks the exact
precondition `Expr::Ref` has, so a future naming scheme is covered without this
crate learning about it.
### a_formula_the_emitter_accepted_evaluates_everywhere
The property the lying test could not state: **if `fitted_formula` says `Ok`, no
cell of the target is a `ref:` error.** It is also what stops the refusal tests
above from passing by refusing everything.
### a_tree_indexing_past_the_spec_says_so
improvise-hkt6. A tree whose `feat` runs past the spec violates the same
precondition in two places, and the two answered differently: `expr_text` said
what was wrong, `mark_features` said `index out of bounds`. Whichever ran first
decided what the caller was told, and `fitted_formula` runs `mark_features`
first — so the legible message was the one nobody ever saw. Driven through
`fitted_formula` because both private functions are reached that way, and the
**message** is asserted rather than merely the panic, since an out-of-bounds
index panics too.
### a_record_added_after_the_fit_is_scored_by_the_formula_alone
The load-bearing half of improvise-7na's answer: scoring a *new* row needs no
saved encoding plan. `fit_csv --predict` calls `encode_row` because its new row
arrives as untyped CSV text; improvise's new row is a record in the same
document, scored through the stored formula from its own measures. So nothing
about the `--predict` path argues for putting a `gp_tree::data::Model` in the
file.
### breeding_improves_on_generation_zero
What rules out a GA whose `breed` returned its input unchanged.
`a_different_seed_fits_a_different_formula` is weak evidence on its own, since
the initial `random_tree` already differs across seeds.
---
## `improvise-gp/src/dataset.rs`
### a_measure_on_the_page_axis_does_not_filter_away_the_training_rows
improvise-vx3. `Workbook::new`'s default view puts `_Measure` on `Axis::Page`,
so `filtering_cats()` named it, `filter_coords` pinned it to the first measure,
and every "record" then carried exactly one measure cell — never the target. The
whole table came back as "no visible record has a Revenue value to learn from"
on a document plainly full of them, which reads as a data problem rather than as
an axis one.
**Every test in the module used `View::new` + `on_category_added`**, under which
a leading-underscore category lands on `Axis::None` — so the fixture was quietly
building the one world the code worked in, and no test could see the default a
real `Workbook` hands out.
The assertion compares the **contents**, not the shape. Pinning only the header
and the row count passes on this fixture even with the bug present: `_Measure`'s
first item is `Revenue`, so every row still has a target and only the feature
columns come back blank — the shape survives while the data does not.
### a_paged_measure_that_is_not_the_target_does_not_empty_the_table
The sharper half: `filter_coords` pinned to the *first* item, so on a document
whose target is not first the table came back empty.
### the_feature_table_is_one_row_per_record_with_the_target_last
The literal row **order** is half of what this pins, and f32 accumulation over
the rows is why: a reordered table makes a seeded fit irreproducible in a way
nothing prints. Since improvise-shyt `matching_cells` yields insertion order, so
the assertion now holds by construction; before it, a test in one process could
not tell "sorted" from "this process happened to hash them in that order".
### a_formula_measure_is_not_a_feature
The fixture lists `Profit` as an explicit `_Measure` item **as well as** making
it a formula target, which is legal and is the only case this test can see: a
target that is only implied by its formula never reaches the explicit item list,
so a fixture without the explicit item passes whether the rule is implemented or
not.
---
## `improvise-gp/src/fit.rs`
Whole-module fixture note — `document()`. **Every category is a `Label`, and
that is the only thing that makes the success path deterministic.** An earlier
version left `Region` regular with a single item and claimed the constant
one-hot had a degenerate `(1, 1)` range that `simplify` would prune. **That
claim is false**: `data.rs::build_categorical` pushes `(0.0, 1.0)` for every
one-hot whatever the column contains, so `simplify` cannot know it is constant
and the champion was free to split on `Region=North`. These tests were passing
on seed luck, and the comment asserting otherwise was the worst part (Copilot on
PR #25). The targets are in the **thousands** so a fit that forgets to
de-normalize is visibly wrong rather than subtly so. The view puts `_Measure` on
`Axis::Page` because that is what `Workbook::new` does — fitting under the real
default is not an extra case, it is the only honest one.
### the_fitted_formula_predicts_in_the_documents_own_units
The de-normalization regression. `fit` runs against a standardized target, so a
champion written down untouched predicts standard deviations: a well-formed
formula, parsing cleanly, evaluating to ±5 where the user's data is 1000–4000.
Nothing downstream can see that — it is a number in a cell.
**Fault injected:** dropping the `denormalize` call in `fit_measure` fails this
and only this test. The loop `panic!`s rather than `continue`s on a non-numeric
cell, because a loop that skips every record asserts nothing and "no cell
evaluated" is itself the improvise-8xhy failure. The claim is then read a second
way off the formula **text**, which does not depend on evaluation at all.
### a_category_and_a_measure_of_the_same_name_are_refused
Copilot on PR #25, and the sharpest finding of the round: asking the *document*
whether it can resolve a feature name is not the same question as asking whether
that name means the column the tree was fitted on. A numeric category called
`Cost` encodes to a feature called `Cost`; with a `_Measure` item of the same
name the emitted `Expr::Ref("Cost")` resolves cleanly — **to the measure** — and
the stored rule silently reads a different column from the one it learned.
**The first version of this test never built the collision.** Adding a category
`Cost` with items puts no `Cost` coordinate on the existing records, so
`feature_table` emits an empty column, `preprocess` drops it, and there was no
duplicate name to find; it passed on the *positional* role bug instead. The
records here carry the coordinate, and the test asserts the fixture really
produces two columns named `Cost` before asserting the refusal.
### an_empty_category_before_a_measure_does_not_break_the_role_check
Copilot on PR #25. `preprocess` `continue`s past an entirely-missing column
without pushing a `ColumnSpec`, so `Dataset::columns` is not index-aligned with
the header once anything is dropped. A positional `i >= category_columns` role
test then read the surviving numeric measure as a category and refused a
perfectly good fit. The test asserts the fixture really drops a column first.
### a_coordinate_is_spelled_as_one_even_when_an_item_shares_the_one_hots_name
The collision a *name* cannot resolve, now closed by construction rather than by
a refusal. The fixture adds a `_Measure` item literally named `Region=North` and
asserts `find_item_category` resolves it — so a check that matched the one-hot's
feature name against the document would have found the wrong column.
Under improvise-4zl7 the one-hot is never spelled by its feature name at all:
`Features::of` reads the **role** off `preprocess`'s plan and emits
`Region IS North`, which cannot collide with any measure however the document is
named. So the assertion is the *absence* of `|Region=North|` in the emitted text
as much as the presence of `Region IS`. Injecting the fault means having
`spell` fall through to `Spelling::Measure` for a `OneHot` role: the fit then
succeeds, the formula reads `|Region=North|`, and it silently evaluates against
the wrong column — which is the original bug wearing the new API.
### a_high_cardinality_numeric_measure_is_one_feature_not_many
The preflight approximates `preprocess`'s type inference, so it can disagree in
both directions. A numeric measure with more distinct values than `MAX_FEATURES`
encodes to **one** feature and must not be refused for having many values.
### a_target_whose_spread_overflows_f32_is_refused_rather_than_panicking
The end-to-end half of improvise-pv3g. Before the fix this test **panicked** at
`crates/improvise-gp/src/lib.rs:288` with `number_text wants a finite value, got
NaN` — the exact crash the ticket reproduced through the shipped binary, and a
hard crash in the TUI rather than a refusal, since `FitMeasure::execute` has no
catch. `document_with_target_scaled_by(1e20)` multiplies only the target's two
levels, so the *spread* is what grows and nothing else about the fixture moves.
The refusal is asked for by its cause — the message must name values "too large
for the 32-bit arithmetic" — so a rival that merely turned `number_text`'s
assertion into an error would not satisfy it, and neither would one that refused
every large document.
### a_target_just_under_the_overflow_still_fits
The half that keeps the guard from being satisfied by refusing everything, which
is the acceptance criterion the ticket wrote for exactly this reason. At `1e18`
the same fixture fits and every emitted leaf is finite; at `1e19` it does not.
Injecting a `Scaling::new` that always returns `None` fails this test and leaves
its sibling passing, which is the separation the pair exists to make.
### a_category_with_a_distinct_value_per_record_is_dropped_not_searched
`FitConfig` bounds the search; nothing bounded the width. A regular category
with one distinct value per record one-hots to one column per record, and
permutation importance is `repeats × rows × features` evaluations on the draw
thread.
**The fixture had to be pushed past `MAX_FEATURES`**, and the assertion names
the *width* outcome specifically. At twelve records this document is refused
too — but by `fitted_formula` noticing the champion split on `Tag=r0`, a
different rule, which would have made the test pass with the guard removed
entirely.
Renamed with improvise-e8no, which changed the outcome from refusing the table
to dropping the column: the assertion is now that the fit lands, that `Tag IS`
is nowhere in the formula, and that `FitOutcome::warnings` names `Tag`. The
cost bound is unchanged — the column is still never searched.
### more_records_than_the_budget_is_refused_before_any_search
The width bound does not bound the work: every stage scales with rows. The
assertion also requires the message to say *what to do* (`Page or Filter`).
### a_document_with_no_usable_feature_is_refused_rather_than_panicking
`preprocess` drops an entirely-missing column, so a document whose only
candidate feature is empty encodes to **zero** features — and `random_tree` then
draws `rng.below(0)`, which `SplitMix64` answers with `0`, and `sample_thresh`
indexes `spec.ranges[0]`. A panic, on a legal document, for any seed that does
not happen to open on a leaf.
This needs the all-`Label` fixture rather than merely an empty measure: with a
regular category present its one-hot survives — range `(0.0, 1.0)` whatever it
holds — and the fit returns a constant leaf instead of crashing.
### a_non_finite_cell_is_refused_rather_than_panicking
A `.improv` cell holding `inf` parses as an `f64`, so this is reachable from a
document rather than only from a bug. `standardize` turns an infinite target
into NaN leaves, an `inf..inf` range makes `sample_thresh` produce a NaN
threshold, and `number_text` then **asserts** — a panic out of a function whose
signature promised a `Result`.
**Verified by disabling the guard, and the two directions differ**: an infinite
*target* really does panic in `number_text` ("wants a finite value, got inf"),
while an infinite *feature* poisons its range and quietly yields a constant
leaf. A panic and a meaningless rule; both are refused.
`NaN` is deliberately absent from the cases: `f64::NAN.to_string()` is `"NaN"`,
which `is_missing` already spells as *missing*, so it is imputed away and never
reaches the search.
### a_name_containing_a_newline_cannot_split_the_status_line
`.improv` pipe quoting makes a newline legal inside a name, so a document can
hold a measure whose name would split the TUI status bar in two.
### a_fit_that_needs_a_category_now_spells_it_as_a_coordinate
improvise-8xhy reaching a real user, and what improvise-4zl7 bought: the same
fixture, the same champion, and a formula instead of a refusal. The fixture
clears **both** measures on every North record: clearing only `Cost` would leave
North carrying both target values, so the one-hot would explain nothing and the
champion would have no reason to split on it.
The second assertion is the one that earns its place. Checking only that the
text says `Region IS` would pass for a rule that routed both groups to the same
leaf — which is what a tree that could not see the coordinate would produce. The
fixture's two groups are 1000 and 4000, so requiring one leaf under 2000 and one
over 3000 separates "spelled it" from "spelled it and meant it".
The `\n`-and-double-space assertions moved with the refusal they guarded; the
one-line rule itself is unchanged and is still asserted by
`a_name_containing_a_newline_cannot_split_the_status_line`.
### the_fold_count_moves_the_champion_and_so_is_recorded
Copilot on PR #25, and it is right: the outcome used to carry `seed` plus
`config`, which is the pair gotcha 37b names — and that pair is **not sufficient
here**. Cross-validation draws from the same generator the fit then draws from,
so changing `cv_folds` alone moves the champion. Recording two of four fields
documented the reproducibility contract without meeting it. The test asserts the
seed and config are *equal* across the two requests first, so the difference can
only be the fold count.
### the_same_seed_under_a_different_config_is_a_different_search
The knob varied is `pop`, not `generations`. More generations over an
exactly-learnable fixture can land on the same optimum and `simplify` normalises
it, so that version of the test would fail one day for a reason that is not a
regression. `pop` changes how many RNG draws each generation makes, so the two
searches diverge from the first one.
### a_fit_named_after_a_category_item_is_refused
Copilot on PR #25. The check used to ask only about *measures*, and `Expr::Ref`
does not: `find_item_category` finds a regular category's item **before**
falling back into `_Measure` (gotcha 7). So naming the fit after some category's
item installed cleanly and then made every later formula referring to it read
the category coordinate. `a_later_formula_referring_to_the_fit_reads_the_fit` is
the property that refusal protects, asserted against a real document.
### a_fitted_rule_at_an_aggregated_cell_routes_per_record_and_sums
The characterization PR #25 left here, now replaced by the rule it was waiting
for. A tree is fitted on individual records; an improvise formula is evaluated
at a `CellKey`, and `evaluate_aggregated` used to aggregate each `Expr::Ref`
*first* and evaluate the `IF` once on the totals. Linear formulas are unaffected
— `SUM(Revenue) - SUM(Cost)` is `SUM(Revenue - Cost)` — and a tree is not, so
the stored rule meant one thing on a record cell and a different thing one row
up. improvise-4zl7's ruling was to aggregate the rule over the members.
The old test asserted the **negative** and carried a message telling whoever
broke it to replace it with the rule. It fired on the first run of the fold.
What this one asserts is the property the ticket names: the aggregated cell
equals the sum of the per-record predictions. That compares two *different*
code paths — a fully-specified key evaluates one member, an aggregating key
folds twelve — rather than one path against itself, but it is still an
end-to-end agreement rather than a hand-computed number. **The hand-computed
evidence is in `improvise-core`**, under
`route_then_aggregate_tests`, on a four-record fixture whose every expected
value is written out in the test name or the assertion.
The third assertion is what stops this one being satisfiable by accident:
routing once on the total lands on a single leaf, so the test refuses a fixture
in which a leaf happens to equal the right answer.
### a_missing_flag_a_date_part_and_a_measures_value_are_refused_differently
improvise-wd6n. `Spelling::Artefact` was one variant standing for **five**
distinct families — `Missing`, `DatePart`, a one-hot over a measure column, the
`(missing)` pseudo-item, and a `Numeric` role over a category column — the last
five reached through a `_ =>` wildcard in `spell`. The single refusal they all
fed said the tree "splits on a `col.missing` flag or a date part", so three of
the five were declined in words naming something they are not. A user fitting
over a text-valued measure column was told about missing-value flags.
The test drives `Features::of` over a real `preprocess` output rather than
fabricating `Spelling`s — which is the point, since what is under test is
`spell`'s classification, and the helper `artefacts()` beside it fabricates
exactly what `spell` decides. That helper now takes the family as an argument
so a fabricated spelling at least names one on purpose; `Features::of` is still
what really decides, and this test is the one that asks it.
Each family's assertion has two clauses, and the second is what earns it: the
error must contain that family's reason **and not the other two's**. The first
clause alone passes against one message naming every family at once, which is
precisely the state the ticket describes. Fault injected: the five arms of
`Artefact::reason` collapsed to one shared string — the test fails on the first
family, and with the injection removed all three arms pass.
The fixture reaches three families from one `preprocess` call (a text measure
column, a measure with a hole in it, a column of ISO dates), which is why the
module is named for what it reaches rather than for "each family". The
remaining two are not reachable from it and are not asserted here.
`AbsentCoordinate` is a permanent refusal; `NumericCategory` was **not**, and
the first draft of this entry and of `gp.md` §11 both called it one — review
found that `fit_measure` simply never tells `preprocess` which columns are
categories. improvise-zv8t has since closed it (below). Worth recording as the
failure mode: the branch that *invented* a classification also wrote the prose
declaring one of its arms unfixable, and nothing but a reader could have caught
that — the suite was green on either reading.
### fit::tests::a_category_whose_items_are_numbers_is_still_spelled_as_a_coordinate
### fit::tests::a_category_whose_items_are_dates_is_still_spelled_as_a_coordinate
improvise-zv8t. `fit_measure` handed `preprocess` an `Overrides::default()`, so
it never said which header columns are document categories — although the very
next line told `Features::of`. `preprocess` infers `Numeric` → `Date` →
`Categorical` from the values, so a **`Year` category with items `2024`/`2025`**
became a numeric column and a **category of ISO dates** became a date column,
and both were refused as encoding artefacts with `Year IS 2024` spellable all
along. User-visible: a fit over such a document could not be stored at all.
No fault had to be injected, because the bug was live. On the unfixed tree both
tests fail inside `fit_measure` with the real refusals, which are the two
arms the ticket named and are worth quoting:
- `["Year"] names a category whose items all parse as numbers, so a reference
to it would resolve to a `_Measure` item of the same name`
- `["Year.epoch_day", "Year.year"] is a part of a date, and date arithmetic is
something improvise does not have anywhere`
The second is why `gp.md` §11's date decline was narrower than it read.
**Why each fixture is discriminating.** `year_document` gives the model **one**
regular category and **no other feature column**, and makes the target a
function of that category alone — so a tree that does not split on `Year` is a
poor fit and the GA does not settle for one. That matters because the refusal is
conditioned on `used_features`: a fit whose champion happened to be a bare leaf
would be *accepted* on the unfixed tree and prove nothing. Asserting
`formula.raw.contains("Year IS")` rather than merely `is_ok()` closes that hole
in both directions. Note what the unfixed tree actually does, because a first
draft of this line got it wrong: it produces **no text at all** —
`fitted_formula` refuses before emitting, which is what the messages quoted
above are. The only unfixed path that emits anything is the bare-leaf champion
named in the previous paragraph, and its text is `SUM(<constant>)`, carrying no
`Year` in any spelling. So the assertion closes the hole; "it would have said
`Expr::Ref(Year)`" was a guess about a branch that does not run.
The numeric test then adds the property the refusal exists to protect: the
formula goes in through `add_formula` and
every record evaluates to a `Number`, not `Error("ref:…")` (improvise-8xhy).
### fit::tests::the_preflight_counts_a_numeric_category_at_its_cardinality
The other half of improvise-zv8t, and the one that is a *cost* rather than a
fix. Forcing a category categorical widens it from one numeric feature to one
per distinct value, so `encoded_width` — the cheap preflight that exists to
refuse a too-wide table **before** `preprocess` materialises it (§14) — had to
learn the same `header[..category_columns]` slice, or it would under-count
exactly the column this ticket is about.
Fault injected: the `col >= category_columns` clause dropped from `encoded_width`,
leaving the pre-ticket inference. The test fails by name, and the failure message
is the evidence — it shows the **exact** check speaking where the preflight
should:
> the PREFLIGHT must refuse it, before anything is materialised: fitting "Score"
> encoded 70 features, over the 64 this host will search.
A test asserting only `expect_err` would have passed under that injection, since
both guards refuse; the discriminator is *which message*, because the two guards
differ in what they have already paid for. The fixture also asserts its own
precondition — that `Year` really is the header's first column — so a
`feature_table` change that reorders the header fails here rather than silently
slicing the wrong names.
### fit::tests::a_wide_numeric_category_is_dropped_rather_than_refusing_the_fit
The **cost** of improvise-zv8t, pinned rather than argued — and it exists
because the first draft of `gp.md` §11 claimed there was none. That draft said
"no document that fitted before stops fitting", reasoning that a tree splitting
on a numeric category was refused as an artefact anyway. The refusal is
conditioned on `used_features`, so a fit that never split on the column
succeeded, and the wide columns are exactly the ones a fit has least reason to
split on.
Measured both ways with a throwaway `#[ignore]` probe before this test was
written, on a fixture of 100 records with a 100-item numeric `Year` beside a
`Cost` that predicts `Score` on its own:
- pre-fix: `FIT LANDS -> |Score (fit)| = SUM(IF(Cost < 48.0150032043457, 970.3…, 3684.7…))`
— `Year` never appears.
- post-fix: `REFUSED -> fitting "Score" would encode 101 features from 100
records`.
So no fault needed injecting; the *change itself* is what the test observes, and
it fails on either side if the behaviour moves. The second half is what keeps it
from being a bare characterization: it marks `Year` a `Label` and asserts the
fit then lands on `Cost`, which is the repair the refusal's own message names.
A test asserting only the refusal would leave that advice unchecked — and advice
in an error message is a promise, so it is the half worth asserting.
The generalisable part is the review finding, not the test: **prose claiming a
change has no cost is a claim, and it is checkable.** The suite was green with
the false sentence in place, exactly as it was green with improvise-wd6n's
"permanent refusal" — the same section, the same failure mode, one paragraph
apart.
**improvise-e8no then closed the cost, and the test was renamed and inverted.**
The budget now declines the *column*: `Year` is added to `Overrides::ignore`,
the fit lands on `Cost`, and the drop comes back in `FitOutcome::warnings`.
The assertion that earns its place is
`repaired.formula.raw == outcome.formula.raw` — ignoring the column and marking
it a `Label` remove the same column, so the feature set and therefore the RNG
draw sequence are identical. That is stronger than "both fit", since a
different feature count moves the champion (gotcha 37(b)).
**What it is NOT is a claim about the pre-zv8t champion, and the difference was
found by printing it.** The drop lands on
`SUM(IF(Cost < 48.0150032043457, 1090.27…, 3684.75…))`: the threshold and the
high leaf are the pre-fix probe's, the low leaf is not (970.3 there). Pre-zv8t
the column was *present* as one inferred numeric feature rather than absent, so
the draw sequence differed — the two agree on the rule and not on the tree. An
earlier draft of this paragraph said the pre-fix champion "is what it lands on
again", which is the same kind of unchecked cost claim this entry was written
to record.
Four rivals were injected and all four fail: guarding nothing, dropping the
*narrowest* category first, dropping every category once over budget, and
dropping silently.
### fit::tests::the_budget_drops_the_widest_category_and_keeps_the_narrow_one
The fixture that separates "widest first" from any other order, and it did not
exist until an injection failed to fail. Every e8no fixture has exactly **one**
over-budget category, so reversing the sort key changed nothing observable —
an injection that does not fail names an untested path, not safe code.
`two_categories_of_different_widths` is 60 records with a 60-item `Wide` and an
8-item `Narrow` beside a numeric `Cost`: 69 columns against a 64 budget. Both
orders get under the budget — dropping `Wide` leaves 9, dropping `Narrow`
leaves 61 — so the outcome that discriminates is **which column survives**, not
whether the fit lands. The test asserts the warnings name `Wide` and do not
name `Narrow`, and its precondition asserts the fixture really is over budget
by the wide column alone, so a `feature_table` change that shrinks it fails
here rather than passing vacuously.
### fit::tests::a_duplicate_column_name_is_refused_before_the_matrix_is_built
Found by adversarial review of improvise-zv8t, and **introduced by it**:
`as_categories` forces by column **name**, while `encoded_width` counts by
column **index**. A document may legally name a regular category and a raw
measure the same thing -- `a_category_and_a_measure_of_the_same_name_are_refused`
already ships that fixture -- and then the *measure* column is forced
categorical by the name match while the preflight still counts it numeric by
index.
Measured on the fixture here (40 records, a 40-item `Dup` category beside a
`Dup` measure): the preflight answers **41** where forcing materialises **80**.
The refusal that catches the document, `no_ambiguous_column_names`, ran *after*
`preprocess`, so the expansion the preflight exists to prevent was allocated and
only then refused. At the default `max_records = 20_000` that is a 20k-row
one-hot matrix built before the error -- `doctrine(guard_the_cost_not_the_result)`
defeated by an inconsistency the ticket itself introduced. The bounded drift
that `doctrine(a_budget_check_is_advisory_or_exact_and_must_say_which)` licenses
is *approximation*, not this.
**The repair is a data dependency, not a moved line.** `as_categories` now runs
the check itself and returns `Result<Overrides>`, so the `Overrides` that
`preprocess` requires cannot exist until the header has been accepted. That is
`doctrine(an_invariant_is_made_unrepresentable_not_validated)` -- the answer
`prune_axis_items`, `Row<T>` and `Checked` each got.
**So the regression guard is the signature, and the injection is a compile
error.** Swapping the two statements back gives
`error[E0425]: cannot find value 'overrides' in this scope`, verified. What that
does *not* cover, stated rather than implied: the check could still be deleted
from `as_categories` deliberately. The accidental reordering -- the way this
actually happened -- is what is now unrepresentable.
The test therefore asserts the **hazard** rather than the ordering: that the
fixture really names one column twice, that the preflight's 41 really disagrees
with what forcing would build, and that `as_categories` itself refuses. A test
asserting only the error message would pass on either ordering, since the
message is identical -- the whole defect was in work done, not in the result.
### the_preflight_counts_the_missing_category_a_forced_column_materialises
improvise-x1ns, found by ChatGPT Codex reviewing PR #90 and live on `main` at
`ce895ff`. `encoded_width` computed `has_missing` and used it in the **numeric**
arm only:
```
true => 1 + usize::from(has_missing),
false => present.iter().collect::<HashSet<_>>().len().max(1),
```
`build_categorical` maps every missing cell to `MISSING_CAT` and includes it in
`distinct(&mapped)`, so a categorical column with N distinct *present* values and
at least one absent cell materialises **N+1** one-hots. The preflight promised N.
It under-reports by exactly one per such column, and it under-reports in the
direction that costs: the preflight runs *before* `preprocess`, the exact check
on `feature_names.len()` runs *after*, so the exact check bounds the error
message and not the allocation — which is the whole reason the preflight exists.
The previous wave's review recorded this shape as "read, not injected" and
"bounded by the exact check"; the second half is false and is now corrected in
`context/code/gp.md` §14.
**Fails before the fix** with
```
the preflight under-reported: it promised 5 features and `preprocess`
materialised 6 — ["Year=(missing)", "Year=2020", "Year=2021", "Year=2022",
"Year=2023", "Cost"]
```
The fixture is Codex's suggested case, and the reason it is the interesting one:
a **forced numeric category with a missing coordinate**. Forcing is what sends a
numeric column down the categorical branch, so only a *forced* column reaches the
arm that was wrong, and only a missing cell makes the arms disagree.
The test does not assert a number. It asks `preprocess` what it actually built
and requires `encoded_width >= data.feature_names.len()`, so it cannot pass by
agreeing with a stale constant, and it keeps holding if the encoding widens for
some other reason. Three preconditions are asserted first, because each would
make it vacuous: that exactly one record lacks the coordinate, that the present
cells really are numeric (otherwise the column would be categorical by inference
and forcing would prove nothing), and that a `Year=(missing)` one-hot is really
among the materialised features.
The `.max(1)` is deliberately outside the addition. A column whose cells are
*entirely* missing has no present values, and `preprocess` skips it with a
warning rather than encoding it; keeping the clamp on the sum leaves that case
counted as 1 exactly as before, so the fix moves only the case it names.
### a_split_on_a_numeric_category_read_as_a_number_cannot_be_stored
The evidence improvise-e8no is declined on, and the only coverage
`Artefact::NumericCategory` has ever had. It was **one of three** families with
no test, not the only one: `MissingFlag` and `DatePart` are covered by
`the_missing_flag_and_the_date_sub_features_are_refused_too`, and
`ValueOfAMeasure` and `AbsentCoordinate` still have none — filed, not fixed
here.
improvise-e8no observes that forcing a document category to `Forced::Categorical`
(improvise-zv8t) widens the encoding, so a high-cardinality numeric category now
exceeds `MAX_FEATURES` where inference used to read it as one numeric column and
the fit succeeded. The decline turns on what that success was worth: a tree that
splits on a numeric column over a *category* column produces a formula
`fitted_formula` **refuses**, because a bare reference to the category's name
would resolve to a `_Measure` item of the same name (improvise-8xhy). So the
pre-zv8t success was either a fit that never touched the column — for which
marking it a `Label` gives a fit over the same remaining features, within budget
— or one that could not be stored. Note what this does *not* establish: an
ordinal encoding is spellable as a weighted indicator sum, so it is declined on
semantics and formula size rather than on spellability (`context/code/gp.md`).
The test asks `preprocess` with `Forced::Numeric`, which is the pre-zv8t reading
reproduced deliberately rather than described, and then asks `Features::of` and
`fitted_formula`. It does **not** use the `artefacts()` helper: that helper
fabricates a `Spelling`, which is the shape gotcha 37(d) warns about — it would
assert that the emitter refuses what the test told it to refuse. Here the
document and the encoder decide.
Injected fault: change `spell()`'s `FeatureRole::Numeric { .. }` arm from
`Spelling::Artefact(Artefact::NumericCategory)` to `Spelling::Measure(column.clone())`
— the "fix" someone would reach for on reading improvise-e8no, and the one that
silently reopens improvise-8xhy. Fails with
```
assertion `left == right` failed: a numeric column over a category column is unspellable
left: Measure("Year")
right: Artefact(NumericCategory)
```
### an_ordinal_split_on_a_category_is_spellable_as_an_indicator_sum
Not a bug fix: this pins a **grammar fact a prose argument rests on**, after that
argument got it wrong. improvise-e8no's decline was first written claiming that
one-hot is the only encoding of a category whose splits the formula language can
spell, and therefore that ordinal encoding was ruled out by spellability. An
adversarial review disproved it by running `parse_formula` instead of reading the
rule that looked relevant: `is_expr` is a `primary`, so it is an `Expr` yielding
1/0, and `cmp_op`'s operands are full `expr`s — so
`IF(2020*(Year IS |2020|) + 2021*(Year IS |2021|) < 2020.5, 1, 2)` parses.
The decline survives on other grounds (a category's items carry insertion order,
not magnitude; and the emitted formula still spells one `IS` per item, so the
width is not saved) — but the *stated reason* was false, and a decline is the one
artefact nothing checks. This test is the check: if the grammar ever narrows so
that an indicator sum stops parsing, the paragraph that says it does becomes
discoverable rather than quietly wrong.
The negative arm is the incidental fact that explains the emitter: `Year IS 2020`
does **not** parse, because `filter_value` is `string | pipe_quoted | bare_ident`
and a bare number is none of them. That is why `formula_text` pipe-quotes a
numeric item name through `quote_name`, and asserting it here keeps the positive
arm from passing for the trivial reason that everything parses.
### the_preflight_counts_the_parts_a_date_measure_expands_into
improvise-x1ns, **second instance** — found by ChatGPT Codex on PR #97, live at
`f128087`, i.e. in the very change that fixed the first one.
`preprocess` infers `Numeric → Date → Categorical` (`data.rs:406`), and
`encoded_width` had **no date arm**: numeric-or-categorical only. A date string
does not parse as `f64`, so a measure column holding dates took the categorical
arm and was counted by its distinct strings. `build_date` emits
`epoch_day/year/month/day/dow` — seven parts once any cell has a time
(`data.rs:326`) — so a column with **one** distinct timestamp promised 1 and
materialised 5.
Fails before the fix, on the plain-date case:
```
a plain date: the preflight under-reported: it promised 4 features and
`preprocess` materialised 8 — ["Region=East", "Region=North", "Region=South",
"When.epoch_day", "When.year", "When.month", "When.day", "When.dow"]
```
**The fixture carries a time case because of the rival fix, not because of the
bug.** Two implementations repair the plain-date failure: "a date column is 5
features", and "5, or 7 when any present cell carries a time". They differ only
on a column containing a time, so the second sub-case is what separates them.
Injected, `date_parts` hard-coded to 5:
```
a date with a time: the preflight under-reported: it promised 8 features and
`preprocess` materialised 10 — [… "When.hour", "When.minute"]
```
Note the plain-date sub-case **passes** under that injection — so without the
time fixture this test would have been satisfied by either reading, and would
have pinned the bug without pinning the rule.
Both arms compare against `data.feature_names.len()` rather than against a width
computed a second way, so the test cannot pass by agreeing with a stale constant
or by reimplementing the thing under test. Two preconditions guard vacuity:
every `When` cell really is the same string (so the categorical count really
would be 1), and the fixture's timestamp really does or does not carry a time,
asked of `parse_datetime` rather than of the literal's shape.
The column must be a **measure** — `as_categories` forces every document
category to `Forced::Categorical`, so a category of dates never reaches the date
branch at all. That is gotcha 37(d)'s "the date decline is narrower than it
read": this is the genuine date *measure*.
### fit::quality_tests::a_huge_lone_importance_is_still_the_whole_of_it
improvise-k844, the arm the ticket's title names. `importance_ranking` computed
`100.0 * imp[j] / total`, which in `f32` **multiplies first**: a lone importance
of `3e37` makes `100.0 * 3e37` overflow to `inf` before the divide ever happens,
and `inf / 3e37` is `inf`. The feature carrying all of the importance was
reported as `inf%` of it. The all-zero early return could not see this — the
total is large and positive, not zero — and neither could
`a_tiny_lone_importance_is_still_the_whole_of_it`, whose whole concern is the
opposite end of the range.
The fault is in the *derived* quantity while every input is finite and ordinary,
which is exactly improvise-pv3g's shape one function over. That is why the
reproduction is `3e37` and not `f32::MAX`: the input is a plausible importance,
not a sentinel.
### fit::quality_tests::shares_are_apportioned_when_the_total_overflows_f32
The silent twin, and the reason the ticket's own proposed fix is not the fix.
Here **no single importance overflows** — two features at `2e38` are each well
inside `f32` — but their `f32` **sum** is `inf`, so every share came back `NaN`
(`100.0 * 2e38` is `inf`, and `inf / inf` is `NaN`). Nothing panicked and
nothing was refused; a user's fit report simply carried `NaN%` against every
feature. This is the same shape as `cv_regress` pooling nothing for a fold it
could not standardize: an overflow that corrupts a reported number with no crash
to announce it.
**The injected rival is the ticket's own title.** Reassociating to
`100.0 * (imp[j] / total)` in `f32` — "multiplies first", fixed by dividing
first — passes `a_huge_lone_importance_is_still_the_whole_of_it` and
`the_shares_sum_to_a_hundred_at_any_scale`, and fails this test and its
non-finite sibling: with `total` already `inf`, `2e38 / inf` is `0.0`, so the
rival turns `NaN%` into a silent **`0%` for every feature**, which is worse,
being readable. Measured: 58 passed, 2 failed under the rival. That separation is
the whole argument for accumulating in `f64` rather than reassociating in `f32` —
`f64` cannot overflow on any sum of finite `f32`s, so the *arithmetic* stops
being able to produce a non-finite share instead of the call site checking for
one afterwards (`doctrine(an_invariant_is_made_unrepresentable_not_validated)`).
### fit::quality_tests::a_non_finite_importance_ranks_nothing
The one case `f64` accumulation cannot absorb: an importance that is *already*
`inf` or `NaN` when it arrives. Summing in `f64` keeps it non-finite, and
apportioning shares of an infinite total is meaningless, so the guard is
`!total.is_finite()` beside the existing `total <= 0.0` and the ranking comes
back empty — nothing to report, rather than `NaN%` against every feature. It
returns data and prints nothing (gotcha 37a). The `NaN` arm is not redundant
with the `inf` arm: `NaN <= 0.0` is `false`, so the pre-existing early return
passed `NaN` straight through.
### fit::quality_tests::the_shares_sum_to_a_hundred_at_any_scale
Extended for improvise-k844 from four scales topping out at `1e6` to six topping
out at `8e37`. `context/code/gp.md` §8 states the contract as "the shares have to
sum to 100% at every scale", and the test stopped eleven orders of magnitude
short of where `f32` actually breaks — so the prose was already right and only
the test was narrow. The top two scales fail before the fix.
Note what this test **cannot** see, which is why the two tests above exist: at
`[8e37, 2.4e38]` the total is finite, so it separates "multiplies first" from
"divides first" and is satisfied by either the rival or the fix.
### fit::tests::a_large_measures_importance_shares_reach_the_report_finite
improvise-k844 end to end, in `improvise-gp`. The reachability is the part the
ticket did not state: both callers measure permutation importance in the
document's **own units** — `fit_csv` through `real_unit_mse`, `fit_measure`
through `mse(&champion, &rows)` against the *unnormalized* rows — so importance
is an MSE difference, which is **squared**. A target near `2e18` therefore yields
an importance near `9e36`, and `100.0 *` that overflows.
That leaves a real window between improvise-pv3g's guard and this one:
`standardize` refuses once `sum((t - mean)^2)` passes `f32::MAX`, around a spread
of `5e18` at twelve rows, while `100.0 * imp` overflows from about `3.4e36`, i.e.
a spread near `1.8e18`. `document_with_score_scaled_by(2e15)` sits inside it —
the fit lands, every value is finite, and before the fix the report read
`Cost reported inf% of the importance: [("Cost", inf)]`. The scale is load-bearing
in both directions: at `4e15` `standardize` refuses and the test fails on
`expect("the fit lands")` instead, proving nothing about shares.
The assertion that the ranking is **non-empty** is there so the test cannot pass
by reporting nothing at all — which is precisely what the non-finite guard makes
`importance_ranking` do, one layer down, if the overflow is ever reintroduced
upstream of it.
### data::tests::a_deserialized_regression_model_cannot_carry_a_non_finite_map
improvise-nzeu, left open deliberately by PR #111 (improvise-pv3g), which made a
non-finite affine map unrepresentable at the point it is **derived** and not at
the point it is **deserialized**. `TargetModel::Regress` carried a raw
`{ mean: f32, std: f32 }` pair, so a hand-edited or corrupted model file
reintroduced the poisoned state on `fit_csv --predict`: `denormalize` computes
`score * inf + mean`, and the NaN surfaced two crates away in `number_text`.
Inbound-only, because `--save` can no longer write one — `standardize` refuses
before a model is produced.
**The poison is ordinary JSON, which is what makes the hole real.** serde_json
will not round-trip `inf` (it writes `null`, which then fails to read back as an
`f32`), so it is tempting to conclude the state is unreachable across the wire.
It is not: `1e39` is well inside `f64`, over `f32::MAX`, and serde's f32 visitor
takes it by `as f32`, which is infinity. A hand editor can plausibly type it and
every parser on the way accepts it.
The test pokes `json["target"]["Regress"][part]` for both `mean` and `std`, and
its first assertion is that the position it pokes currently holds a **finite
number** — which is simultaneously the "assert the mutated input is still
well-formed" discipline and the guard on the saved shape. It asserts the error
*text* names finiteness rather than merely that deserialization failed, because
a rewrite that broke the JSON would fail too and prove nothing.
Three rivals, each caught at a different assertion:
- **Plain `derive(Deserialize)` on `Scaling`** — the state at HEAD — fails at
`a non-finite mean was accepted`.
- **A `try_from` that checks `std` only** fails the same way, on `mean`. This is
the rival that matters for `context/code/gp.md` §7's own note that the `mean`
arm of `Scaling::new` is a contract of the type with no reachable path:
deserialization *is* that path, so the arm now has a test that fails when it is
removed.
- **Dropping `#[serde(flatten)]`** fails at the shape assertion with
`"mean" ... : null`, since the map would then nest under a `scaling` key and
every previously saved model would stop loading.
### fit::tests::a_budget_notice_precedes_a_table_warning_which_precedes_a_preprocess_one
improvise-ipqu. `FitOutcome::warnings` is three tiers concatenated by two
`splice(0..0, ..)` calls in `fit_measure`, and the resulting order — the
budget's notices, then the feature table's, then `preprocess`'s — is stated in
the field's own doc comment **and** in `context/code/gp.md` §14, and is joined
into a user-facing status line. It was checked nowhere. Swapping the two
statements left all 74 `improvise-gp` tests green, measured before and after.
This is the shape gotcha 43 names — a rule "held up only by the order of two
statements" — and the repair is the same one
`a_published_basis_follows_the_grid_updates_it_describes` made: inject the swap
and watch a test fail.
**No pre-existing fixture produced more than one tier**, which is why nothing
caught it, and building one is the whole of the work. `all_three_warning_tiers_at_once`
extends `two_categories_of_different_widths` — 60 `Wide` items against a
64-feature budget, so `Wide` is dropped and a notice is emitted — with two
further things, each aimed at exactly one of the other tiers. A `Blank` item on
`_Measure` carrying **no cell anywhere** is a header column (`raw_measures`
reads item names, not data) that is empty in every row, which is what
`preprocess` warns about. And a `Cost` cell at `(w0, n1)`, a coordinate the base
fixture does not use, is one visible record with no `Score` value, which
`feature_table` skips and warns about. Neither addition disturbs the budget
arithmetic: the skipped record contributes no row, so `Wide` is still 60 wide,
and `Blank` costs 2, leaving `60 + 8 + 1 + 2 = 71` over budget by `Wide` alone.
Asserting that the concatenation *contains* all three would be vacuous
(`doctrine(fold_tests_must_test_order_not_the_axiom)`), so the assertion is on
the three positions. The helper resolving a tier to its index **panics** when a
tier is absent rather than returning a sentinel, so a future change that stops
one tier speaking fails here loudly instead of making the ordering assertion
trivially true over two elements.
Two rivals, each moving a different boundary, and each caught only by this test
(74 passed, 1 failed, both times). Swapping the two `splice` statements gives
table → notice → preprocess and fails at `notice < table`. Replacing the first
`splice(0..0, table.warnings)` with `extend(table.warnings)` gives notice →
preprocess → table and fails at `table < preprocessed` — which is the rival that
earns the third tier, since the swap alone never moves `preprocess`'s.