diff --git a/context/code/core.md b/context/code/core.md index c3de55fb..f5d5abf0 100644 --- a/context/code/core.md +++ b/context/code/core.md @@ -677,9 +677,20 @@ because the reader is display-only and four call sites would have to thread one. **What the bug was, and why six tests sat on top of it.** The precision was read as `fmt[i+1..].parse::()` over the *whole* tail, so `"2f"` failed to parse and fell through to `unwrap_or(0)` — `,.2f` displayed **zero** decimals, in the -TUI, in the browser projection and in CSV export. `",.2f"` is the spelling six -tests in this workspace reach for, and none could see it: they assert the stored -*string* round-trips, or — in `export_csv_quotes_commas_per_rfc4180` — that the -rendered value `starts_with("1,200")`, which `1,200` and `1,200.00` both satisfy. -A prefix check over a value whose *suffix* is the thing in question answers a -neighbouring question, which is why that one is now an exact equality. +TUI, in the browser projection and in CSV export. `",.2f"` and its `,.3f`/`,.4f` +siblings are what this workspace's own fixtures reach for, in about ten tests, +and none could see it: they assert the stored *string* round-trips or use it as +an opaque value to dirty a widget, or — in `export_csv_quotes_commas_per_rfc4180` +— they assert the rendered value `starts_with("1,200")`, which `1,200` and +`1,200.00` both satisfy. A prefix check over a value whose *suffix* is the thing +in question answers a neighbouring question, which is why that one is now an +exact equality. + +**Only two of the three consumers are guarded.** Reverting the fix fails +`format.rs`'s own unit tests and `export_csv_quotes_commas_per_rfc4180`, and +nothing else in the workspace: `ui/project.rs::grid_of` (the browser projection) +and `ui/app.rs` both call `parse_number_format` and neither has a test that +renders a cell and asserts its digits. That gap is named rather than closed — +those files are `improvise-command`, which was another wave's ground when this +landed — and it is the more useful half of this entry, because it says where the +next regression would go unseen. diff --git a/context/code/gp.md b/context/code/gp.md index f8b76e29..65e7667d 100644 --- a/context/code/gp.md +++ b/context/code/gp.md @@ -595,7 +595,9 @@ The previous wave's review saw a version of this and reasoned it away as *"read, not injected"* and *"bounded by the exact check"*. Both halves of that are the failure mode this file exists to catch: the first declined to run the experiment, and the second repeated the sentence above instead of checking it. **An argument -that a cost is bounded is checkable, and this one was false for eight months.** +that a cost is bounded is checkable, and this one was checkable the day it was +written**: `encoded_width` landed 2026-09-19 and the sentence 2026-09-20, so the +code it described was one commit old and one file away. `encoded_width` remains **advisory** in the direction that is safe: it may over-count, and a column whose cells are *entirely* missing is counted as one @@ -631,34 +633,69 @@ rather than on what the document says. But the stronger reason to decline is tha the alternatives are not merely distasteful — they are **unstorable**, and the code already says so. -The formula language's only coordinate predicate is `is_expr`, spelled -`identifier ~ sp+ ^"IS" ~ sp+ filter_value` in `formula.pest`: a **single-value -equality**. There is no `IN`, no ordering over a category's items, and no way to -put a coordinate on either side of a `cmp_op` — a coordinate becomes an `Expr` -only by passing through `IS`. So the set of predicates a fitted tree may state -about a category is exactly the set of its items, one at a time, and **one-hot is -the only encoding whose splits are spellable**. The width is not an -implementation's appetite; it is the size of the vocabulary. +A coordinate becomes an `Expr` **only by passing through `IS`**: `is_expr` is +`identifier ~ sp+ ~ ^"IS" ~ sp+ ~ filter_value`, a single-value equality, and +there is no `IN` and no ordering over a category's items. (`where_clause` and +`inline_where` are coordinate predicates too, and also single-value equalities — +but they *gate* a formula or an aggregate rather than being an `Expr`, which is +gotcha 37(d)'s distinction and is why `IS` is the one that matters here.) So +whatever a tree says about a category, the emitted formula names its items **one +at a time**. -That disposes of the two representations the ticket invites. **Ordinal** encoding -imposes an order the document does not assert, and a split on it is already a -classified refusal: `spell()` maps `FeatureRole::Numeric` over a *category* -column to `Artefact::NumericCategory`, whose reason is that a reference to the -category's name would resolve to a `_Measure` item of the same name — a different -column (improvise-8xhy). **Hashing** is strictly worse: a bucket has no name in -the document at all, so it cannot even be refused informatively. +**A first draft of this paragraph said that made one-hot the only spellable +encoding. That is false**, and an adversarial review disproved it by running +`parse_formula` rather than reading the grammar: `is_expr` is a `primary`, so it +is an `Expr` yielding 1/0, and `cmp_op`'s operands are full `expr`s — so an +ordinal split *is* spellable, as a weighted indicator sum +(`IF(2020*(Year IS |2020|) + 2021*(Year IS |2021|) < 2020.5, …)`). The claim is +recorded here rather than quietly deleted because it is the shape this file keeps +catching: a grammar argument made by reading the rule that looked relevant +instead of running the parser. + +So **ordinal is not ruled out by spellability, and is ruled out anyway**, for two +reasons that survive. It invents a **metric the document does not assert** — a +category's items carry insertion order, not magnitude, so `Year < 3.5` means "the +first four items as they happen to be ordered" and stops meaning that when +someone reorders them. And the width it saves does not stay saved: the *encoding* +would be one feature, but the *formula* still spells one `IS` per item, so it +trades a `MAX_FEATURES` problem for a formula-size problem while adding a false +order. What the current emitter does with it is narrower and is not the argument: +`spell()` maps `FeatureRole::Numeric` over a *category* column to +`Artefact::NumericCategory`, because a bare reference to the category's name +would resolve to a `_Measure` item of the same name (improvise-8xhy) — a fact +about the bare-ref spelling, not about ordinals in general. **Hashing** is ruled +out outright: a bucket has no name in the document, so it cannot even be refused +informatively. And this is what makes the decline clean rather than resigned. The "fit that previously succeeded" was one of two things, never a third. Either the tree **never split** on the numeric category — in which case the column contributed -nothing, and marking it a `Label` yields the same fit, honestly, inside the -budget, which is exactly what the refusal already advises. Or the tree **did** -split on it, and `fitted_formula` refused the result as `Artefact::NumericCategory` -at storage time. **No fit is lost that was both storable and used the column**, -which is asserted rather than argued, by -`a_split_on_a_numeric_category_read_as_a_number_cannot_be_stored`. That test had -no predecessor: `NumericCategory` was the one artefact family with no coverage, -and the decline rests on it. +nothing to the rule, and marking it a `Label` gives a fit over the same remaining +features, inside the budget, which is exactly what the refusal already advises. +Or the tree **did** split on it, and `fitted_formula` refused the result as +`Artefact::NumericCategory` at storage time. So **no fit is lost that was both +storable and used the column**. + +Two things that claim less than a first draft of this paragraph did, both worth +keeping. Marking the category a `Label` does **not** yield *the same fit*: the +feature count changes, so the RNG draw sequence in `random_tree`/`sample_thresh` +changes, so the champion for a given seed changes — gotcha 37(b)'s own rule, +applied to this paragraph. "The same fit" is a checkable cost claim and it is +false; what is true is a fit over the same remaining features. And the dichotomy +above is **argued**, from `spell()`'s exhaustive match over `FeatureRole`, not +asserted: what +`a_split_on_a_numeric_category_read_as_a_number_cannot_be_stored` asserts is one +instance of its second branch, over one three-row fixture. That is the most a +test can do here, and saying so is the difference between evidence and a +flourish. + +The test still had no predecessor. `NumericCategory` had no coverage before it — +**and it was one of three, not the only one**: grepping each `Artefact` variant +and each variant's `reason()` text, `MissingFlag` and `DatePart` are covered by +`the_missing_flag_and_the_date_sub_features_are_refused_too`, and +**`ValueOfAMeasure` and `AbsentCoordinate` have no test at all**, before this +change or after it. Two families remain uncovered; that is filed, not fixed +here. What the decline does cost, stated rather than waved at: a user whose `Year` category really is a quantity they want fitted **as a number** has no way to say diff --git a/context/test-evidence/formula-and-io.md b/context/test-evidence/formula-and-io.md index 9704b136..bcfaed99 100644 --- a/context/test-evidence/formula-and-io.md +++ b/context/test-evidence/formula-and-io.md @@ -455,10 +455,6 @@ into two fields, **corrupting every row**. The test reads the file back with a real CSV reader and asserts the field *count*, which is the thing a string `contains` check cannot see. ---- - -## `improvise-io/src/import/analyzer.rs` - **Strengthened by improvise-711n.** The assertion on the value was `records[1][1].starts_with("1,200")`, and the fixture's format is `,.2f` — so the check could not tell `1,200` from `1,200.00` and sat green while @@ -467,6 +463,11 @@ the `format.rs` fix fails it with `left: "1,200"`, `right: "1,200.00"`, which is the cross-crate demonstration that the format bug reached a user's exported file and not only the screen. + +--- + +## `improvise-io/src/import/analyzer.rs` + ### detect_short_year_format Two-digit years are ambiguous with the four-digit format, so `%m/%d/%Y` matches diff --git a/context/test-evidence/gp.md b/context/test-evidence/gp.md index 07aec34f..beaa3c77 100644 --- a/context/test-evidence/gp.md +++ b/context/test-evidence/gp.md @@ -803,19 +803,24 @@ 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 the one artefact family with no -test, which is why the decline needed writing before it could be made. +`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 reference to the category's name would -resolve to a `_Measure` item of the same name (improvise-8xhy). So the +`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` is exactly equivalent and within budget — or one that could -not be stored. +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 @@ -834,3 +839,27 @@ assertion `left == right` failed: a numeric column over a category column is uns 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. diff --git a/crates/improvise-gp/src/fit.rs b/crates/improvise-gp/src/fit.rs index 36a14c32..b00d8fa8 100644 --- a/crates/improvise-gp/src/fit.rs +++ b/crates/improvise-gp/src/fit.rs @@ -267,8 +267,8 @@ pub fn fit_measure( cannot spell and a search cannot minimise. Correct or clear those cells first" ); - // The exact width, now that it is known. The preflight above is advisory, and - // this is the check that enforces the bound in both directions of a drift. + // The exact width, now that it is known. It bounds the error MESSAGE, not the + // cost: `preprocess` has already run — the preflight is the cost bound (§14). anyhow::ensure!( data.feature_names.len() <= MAX_FEATURES, "fitting {target:?} encoded {} features, over the {MAX_FEATURES} this host will \ diff --git a/crates/improvise-gp/src/lib.rs b/crates/improvise-gp/src/lib.rs index 01837dc4..cd48b58b 100644 --- a/crates/improvise-gp/src/lib.rs +++ b/crates/improvise-gp/src/lib.rs @@ -812,6 +812,29 @@ mod tests { } } + /// improvise-e8no declines ordinal encoding on semantics, NOT on + /// spellability — a first draft said otherwise. `context/code/gp.md`. + #[test] + fn an_ordinal_split_on_a_category_is_spellable_as_an_indicator_sum() { + // `is_expr` is a `primary`, so it is an `Expr` yielding 1/0, and + // `cmp_op`'s operands are full `expr`s. + for text in [ + "Score = (Year IS |2020|) + (Year IS |2021|)", + "Score = IF(2020*(Year IS |2020|) + 2021*(Year IS |2021|) < 2020.5, 1, 2)", + ] { + assert!( + parse_formula(text, MEASURE).is_ok(), + "the grammar the decline reasons about must really accept {text:?}" + ); + } + // And the reason a numeric item name must be pipe-quoted: a bare + // number is not a `filter_value`. + assert!( + parse_formula("Score = (Year IS 2020)", MEASURE).is_err(), + "a bare number as a filter_value must stay a parse error" + ); + } + /// improvise-e8no's decline rests on this: the pre-zv8t encoding of a /// numeric category yields fits no document can store. Evidence: `gp.md`. #[test]