f187fa57fe
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
1263 lines
68 KiB
Markdown
1263 lines
68 KiB
Markdown
# 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.
|