review(gp): the zv8t prose claimed a cost it has, and the cost is measurable
Self-review of 99cfb7f, of the prose rather than the diff. §11 said the forcing
cost nothing — "no document that fitted before stops fitting" — reasoning that a
tree splitting on a numeric category was refused as an artefact anyway.
That is false. The refusal is conditioned on `used_features`, so a fit that
simply never split on the numeric category SUCCEEDED, and the wide categories
are exactly the ones a fit has least reason to split on. Measured with a
throwaway #[ignore] probe on both sides of the fix — 100 records, a 100-item
numeric `Year` beside a `Cost` that predicts `Score` on its own:
pre-fix: FIT LANDS -> SUM(IF(Cost < 48.0150032043457, 970.3.., 3684.7..))
post-fix: REFUSED -> would encode 101 features from 100 records
So the sentence one paragraph below improvise-wd6n's corrected "permanent
refusal" was the same failure mode: a claim nothing in the suite could check,
green either way. Noting it because the pattern, not the sentence, is the
finding — `doctrine(prose_that_calls_something_permanent_forecloses_the_ticket)`
has a sibling in prose that calls something free.
The trade is still right, for a reason that is NOT "both refuse": the old
success was accidental, since `Year` was encoded as a continuous number over
1900–1999 that the fit could only have used by producing a formula it would then
be refused for. What is genuinely lost is the case where the user wanted
neither — a wide numeric category left alone while the fit uses the measures.
Filed as improvise-e8no (P3), with the acceptance criterion that the fix is a
budget that can decline A COLUMN rather than the whole table, and an explicit
"do not close this by making the forcing conditional on cardinality", which
would be inference by another name.
`a_wide_numeric_category_now_refuses_a_fit_that_used_to_ignore_it` pins both
halves. Its second half is what keeps it from being a bare characterization: it
marks `Year` a Label and asserts the fit then lands on `Cost`, because advice in
an error message is a promise and this is the half worth asserting.
Note for the record: 99cfb7f's own message carries the false claim and cannot be
corrected. Per design-principles §6, the correction lives in context/ and this
says so — the message is superseded by §11 and by the test-evidence entry.
1463 passing, up from 1462.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
+31
-10
@@ -404,16 +404,37 @@ reachable from `fit_measure`, which is the only production entry point; it is
|
||||
not unreachable code, and deleting it would be deriving the classification from
|
||||
one caller's habits instead of from the roles.
|
||||
|
||||
**Forcing widens the encoding, deliberately.** A `Year` category with two items
|
||||
was one numeric feature and is now two one-hots; with two hundred items it is
|
||||
two hundred, and `MAX_FEATURES` refuses it — where before it encoded as a single
|
||||
feature and was refused one stage later as an artefact. Both refuse, so no
|
||||
document that fitted before stops fitting; what changes is *which* guard speaks,
|
||||
and the width guard's message names the repair (mark the category a Label). The
|
||||
preflight `encoded_width` had to learn the same slice for that to be true, or it
|
||||
counted such a column as one feature and handed the refusal to the exact check
|
||||
after materialising the matrix — which is the side of the cost the preflight
|
||||
exists to stay on (§14).
|
||||
**Forcing widens the encoding, and that costs a document.** A `Year` category
|
||||
with two items was one numeric feature and is now two one-hots; with a hundred
|
||||
items it is a hundred, and `MAX_FEATURES` refuses the fit outright.
|
||||
|
||||
The first draft of this paragraph claimed "no document that fitted before stops
|
||||
fitting", on the reasoning that a tree splitting on such a column was refused as
|
||||
an artefact anyway. **That is false, and the review that caught it was of this
|
||||
prose rather than of the diff.** The refusal is conditioned on `used_features`,
|
||||
so a fit that simply *never split* on the numeric category succeeded — and it
|
||||
is precisely the wide categories that a fit has least reason to split on.
|
||||
Measured, on a hundred records with a hundred-item numeric `Year` beside a
|
||||
`Cost` that predicts `Score` on its own: before, the fit lands on
|
||||
`IF(Cost < 48.0…)` and `Year` is never touched; after, the preflight refuses
|
||||
at 101 features. `a_wide_numeric_category_now_refuses_a_fit_that_used_to_ignore_it`
|
||||
pins both halves, including that the repair the message names really works.
|
||||
|
||||
It is still the right trade, for a reason that is *not* "both refuse". The old
|
||||
success was accidental: `Year` was encoded as a single continuous number over
|
||||
1900–1999, an encoding that means nothing and that the fit could only use by
|
||||
producing a formula it would then be refused for. The document now gets a
|
||||
refusal that names the repair — mark the category a Label — and marking it
|
||||
restores the pre-zv8t feature set exactly. What is lost is the case where the
|
||||
user wanted neither: a wide numeric category left alone while the fit uses the
|
||||
measures. That is filed as **improvise-e8no**; the honest fix is a budget that
|
||||
can decline *a column* rather than the whole table, which is a change to what
|
||||
`encoded_width` returns and not to this decision.
|
||||
|
||||
The preflight `encoded_width` had to learn the same slice, or it counted such a
|
||||
column as one feature and handed the refusal to the exact check *after*
|
||||
materialising the matrix — which is the side of the cost the preflight exists
|
||||
to stay on (§14).
|
||||
|
||||
**And the refusal's own words must not name a column kind the arm does not
|
||||
know.** `FeatureRole::Missing` is *not* conditioned on `is_category` — unlike
|
||||
|
||||
@@ -671,3 +671,35 @@ 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_now_refuses_a_fit_that_used_to_ignore_it
|
||||
|
||||
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. See improvise-e8no for the case the trade genuinely loses.
|
||||
|
||||
@@ -648,6 +648,75 @@ mod tests {
|
||||
])
|
||||
}
|
||||
|
||||
/// The cost improvise-zv8t pays, pinned rather than argued: a wide numeric
|
||||
/// category that a fit used to IGNORE is now refused. `context/code/gp.md` §11.
|
||||
#[test]
|
||||
fn a_wide_numeric_category_now_refuses_a_fit_that_used_to_ignore_it() {
|
||||
let (model, view) = wide_category_beside_a_good_measure();
|
||||
let err = fit_measure(&model, &view, "Score", FIT_INTO, &request())
|
||||
.expect_err("101 one-hots is over the width budget");
|
||||
let msg = err.to_string();
|
||||
assert!(msg.contains("would encode 101 features"), "{msg}");
|
||||
// The message must name the repair, because this document fitted before:
|
||||
// marking `Year` a Label restores the pre-zv8t encoding exactly.
|
||||
assert!(msg.contains("mark such a category as a Label"), "{msg}");
|
||||
let (mut model, view) = wide_category_beside_a_good_measure();
|
||||
model.category_mut("Year").expect("Year exists").kind = CategoryKind::Label;
|
||||
let outcome = fit_measure(&model, &view, "Score", FIT_INTO, &request())
|
||||
.expect("the repair the message names must actually work");
|
||||
assert!(
|
||||
outcome.formula.raw.contains("Cost"),
|
||||
"the fit must land on the measure it always used: {:?}",
|
||||
outcome.formula.raw
|
||||
);
|
||||
}
|
||||
|
||||
/// 100 distinct numeric `Year` items beside a `Cost` that predicts `Score`
|
||||
/// on its own — so a fit has no reason to touch `Year` at all.
|
||||
fn wide_category_beside_a_good_measure() -> (Model, View) {
|
||||
let mut model = Model::new("sales");
|
||||
model.add_category("Year").expect("under the limit");
|
||||
model.add_category(TAG).expect("under the limit");
|
||||
model.category_mut(TAG).expect("just added").kind = CategoryKind::Label;
|
||||
for i in 0..100u32 {
|
||||
model
|
||||
.category_mut("Year")
|
||||
.expect("just added")
|
||||
.add_item(format!("{}", 1900 + i));
|
||||
model
|
||||
.category_mut(TAG)
|
||||
.expect("just added")
|
||||
.add_item(format!("r{i}"));
|
||||
}
|
||||
let m = model.category_mut(MEASURE).expect("_Measure always exists");
|
||||
for item in ["Cost", "Score"] {
|
||||
m.add_item(item);
|
||||
}
|
||||
for i in 0..100u32 {
|
||||
let cost = f64::from(i);
|
||||
for (measure, value) in [
|
||||
("Cost", cost),
|
||||
("Score", if cost > 50.0 { 4000.0 } else { 1000.0 }),
|
||||
] {
|
||||
model.set_cell(
|
||||
CellKey::new(vec![
|
||||
("Year".to_string(), format!("{}", 1900 + i)),
|
||||
(TAG.to_string(), format!("r{i}")),
|
||||
(MEASURE.to_string(), measure.to_string()),
|
||||
]),
|
||||
CellValue::Number(value),
|
||||
);
|
||||
}
|
||||
}
|
||||
let mut view = View::new("Default");
|
||||
for cat in model.category_names() {
|
||||
view.on_category_added(cat);
|
||||
}
|
||||
view.set_axis("Year", Axis::Row);
|
||||
view.set_axis(MEASURE, Axis::Page);
|
||||
(model, view)
|
||||
}
|
||||
|
||||
/// gotcha 37b: the seed alone does not reproduce a fit, so the outcome
|
||||
/// carries the config beside it — and the pair really is sufficient.
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user