Merge pull request #87 from fiddlerwoaroof/wave/harness-generator-gp

Wave G: the generator's real cost, the artefact families, and two harness gaps
This commit is contained in:
Ed Langley
2026-09-20 22:10:19 -07:00
committed by GitHub
10 changed files with 391 additions and 35 deletions
+16
View File
@@ -810,6 +810,22 @@ the keyboard disagreed. `the_formula_panel_has_no_enter_to_mirror_and_the_view_p
asserts the binding's absence and names `click_formula_row` as what must change asserts the binding's absence and names `click_formula_row` as what must change
if it ever appears. if it ever appears.
**The browser assertion that PR #60 dropped stays dropped, and improvise-qf57
is the decision rather than the oversight.** `#grid input` `toHaveCount(0)` was
argued to be redundant with the `#mode` assertion on the grounds that
`toHaveText("FORMULAS")` is exact and `AppMode::FormulaEdit` labels as
`"FORMULA"` — a coincidence of two strings differing by one character, which
would be a thin reason to leave it out. Measuring it gives a better one: it is
not redundant, it is **unreachable**. `web/app.js` creates a grid `<input>` in
exactly one place, the grid cell's own `mousedown`, gated on `cell.editable`;
no repaint creates one and no panel handler can reach it. And an invented
formula-row activation opens the **minibuffer**, not a grid editor. Injecting
`Command::ChangeMode(AppMode::formula_edit())` into `click_formula_row` and
running the spec: `#grid input` `toHaveCount(0)` **passes**, and the run fails
on the `#mode` assertion below it. Restoring it would add an assertion that
cannot fail — the same defect this file already records one line down, where an
explanatory `assert!` was removed for never being able to be the failing one.
**Rule 2 — a click is all of its intents or none of them** (improvise-hkr8). **Rule 2 — a click is all of its intents or none of them** (improvise-hkr8).
Each `click_*` returns `Option<Vec<Command>>` and `click` does the single Each `click_*` returns `Option<Vec<Command>>` and `click` does the single
`unwrap_or_default`. The case that matters is an activation the client cannot `unwrap_or_default`. The case that matters is an activation the client cannot
+69 -8
View File
@@ -289,7 +289,19 @@ never by the shape of the name:
|---|---|---| |---|---|---|
| `Numeric` | a measure | `Expr::Ref(measure)` | | `Numeric` | a measure | `Expr::Ref(measure)` |
| `OneHot { value }` | a category, `value != "(missing)"` | `Cat IS Item` | | `OneHot { value }` | a category, `value != "(missing)"` | `Cat IS Item` |
| anything else | | `Artefact` | | `Numeric` | a category | `Artefact(NumericCategory)` |
| `DatePart` | | `Artefact(DatePart)` |
| `Missing` | | `Artefact(MissingFlag)` |
| `OneHot` | a measure | `Artefact(ValueOfAMeasure)` |
| `OneHot { "(missing)" }` | a category | `Artefact(AbsentCoordinate)` |
The last five arms were **one** `_ => Artefact` until improvise-wd6n, and the
wildcard was not merely untidy. The refusal it fed read "a `col.missing` flag or
a date part is an encoding artefact", so three of the five families were refused
in words naming something they are not — and a future `FeatureRole` variant
would have joined them silently, declined under whichever family the message
happened to mention. The match is now exhaustive, so a sixth role is a compile
error rather than a sixth thing that message misdescribes.
The two exclusions are not fussiness. A one-hot over a **measure** column names The two exclusions are not fussiness. A one-hot over a **measure** column names
no coordinate at all — the record's value for that measure is text, and no coordinate at all — the record's value for that measure is text, and
@@ -326,14 +338,63 @@ flags stay covered without a second copy of `preprocess`'s naming rules.
`Artefact` reaching it is a precondition failure and panics, the same split `Artefact` reaching it is a precondition failure and panics, the same split
`number_text` makes for a non-finite value. `number_text` makes for a non-finite value.
### The two thirds this still does not close ### The two families improvise-wd6n declines, separately
`col.missing` means "this record has no value for measure X" — a different Both stay `Artefact` and both stay refused. The decline is written here rather
vocabulary, and one the formula language has no word for at all. The date than left open, because each needs a **formula-language** change and neither
sub-features (`when.year`, `when.month`) need date arithmetic improvise does not needs an emitter change — and they are answered separately because the
have. Both stay `Artefact`, both stay refused, and both are filed as vocabulary each wants is different. What changed with the decline is that the
improvise-wd6n rather than smuggled in — separately from each other, because refusal now says which, in that family's own words.
they need different vocabulary.
**`col.missing` is declined because the language has no word for absence.** The
flag means "this record has no value for measure X". `Expr::Ref` over an absent
cell is an `Error`, not a zero, so there is no existing spelling to reuse and no
way to phrase the question as an expression. Closing it means deciding what an
absence *is* in a language whose aggregates already treat it as "no rows" — and
that decision reaches `apply_agg`, `eval_agg` and the text-unanimity rule, not
just this emitter. It is not worth that for a split a fit happens to like.
**The date sub-features are declined because improvise has no dates.**
`when.epoch_day`, `.year`, `.month`, `.day`, `.dow`, and with a time component
`.hour`, `.minute` — `gp_tree::data::build_date` emits them. A `.improv` cell
holding a date is `CellValue::Text`; nothing in `eval_expr` reads one, and there
is no date type, no date literal and no date arithmetic anywhere in the crate.
Closing this is a type addition to the value language, and the sub-features are
the *smallest* part of it: a user who could split on `when.year` would
immediately want to filter and aggregate by it.
Both are visible to the user only as a refusal naming the feature and the
reason, which is the right interim: a fit that cannot be stored says so, rather
than storing a formula whose every cell is a reference error (improvise-8xhy).
`improvise-4zl7`'s route-then-aggregate rule (`context/code/core.md` §13) is
untouched — nothing new is emitted, so there is nothing new for it to hold over.
Two of the other three are declined permanently: a one-hot over a measure and
the `(missing)` pseudo-item name no coordinate *by construction*, so no
vocabulary closes them. They are refusals rather than gaps.
**`NumericCategory` is not, and this paragraph said it was** — which is the
more useful half of improvise-wd6n, because prose that calls something a
permanent refusal is what stops anyone filing the ticket. `fit_measure` hands
`preprocess` an `Overrides::default()`, so it never says which header columns
are document categories, although the very next line tells `Features::of`.
`preprocess` then infers `Numeric` → `Date` → `Categorical` from the cell
values, so a `Year` category with items `2024`/`2025` becomes a *numeric column*
and a category of ISO dates becomes a *date column* — and both are refused,
`Year IS 2024` being exactly spellable all along. That needs no
formula-language change whatever: `Forced::Categorical` already exists and is
the knob. Filed as **improvise-zv8t**. The same accident is why a `DatePart`
can arrive over a category column, so the date family's decline above is
narrower than it looks: it covers a genuine date *measure*, not a category
misread as one.
**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
its `Numeric` and `OneHot` neighbours — and `dataset.rs` writes `""` for a
record with no coordinate for a category, so a `.missing` flag over a
**category** is reachable. Its clause therefore says "that column" and not
"that measure": naming the wrong kind there would be this section's own charge
— declined in words naming something they are not — surviving one level down.
## 12. A fit is persisted as its formula, and nothing else (improvise-7na) ## 12. A fit is persisted as its formula, and nothing else (improvise-7na)
+26 -3
View File
@@ -537,9 +537,13 @@ file server — **deterministic where a closed port is not**: a refused port's
`close` timing is a property of the host's network stack, and on the GitHub `close` timing is a property of the host's network stack, and on the GitHub
runner it did not arrive inside the window. The page cannot tell the two apart runner it did not arrive inside the window. The page cannot tell the two apart
and neither can `web/app.js`: in both cases the socket never opened. and neither can `web/app.js`: in both cases the socket never opened.
`HANDSHAKE_TIMEOUT_MS` restates `web/app.js`'s own constant, because two tests `HANDSHAKE_TIMEOUT_MS` is **imported from `web/shell.js`** and restates nothing
have to outlast it and a number that silently disagreed with the page would make (PR #65), and **one** test outlasts it — this sentence said the opposite of both
them pass for the wrong reason. `logStatus` records **every distinct `#status`**, for a while, which is gotcha 48's third clause: a fact stated twice is changed by
a grep, not an edit. Since improvise-669b the spec also compares the *running
page's* window against the one it imported, because the single home is in the
TREE and the page loads `harness/serve/app/shell.js` — see
`outlastTheWatchdog` below. `logStatus` records **every distinct `#status`**,
not just the last one, because several of these tests are about a message being not just the last one, because several of these tests are about a message being
*replaced*, which the final value cannot see. *replaced*, which the final value cannot see.
@@ -624,6 +628,25 @@ watchdog never given the chance — vacuous, and green. `outlastTheWatchdog` rea
moved. Demonstrated both ways: with the install removed the suite passed 6/6, moved. Demonstrated both ways: with the install removed the suite passed 6/6,
and with the guard in place the same omission fails by name. and with the guard in place the same omission fails by name.
**And it guards its second premise since improvise-669b: that the page runs the
constant the spec imported.** PR #65 single-homed the handshake window in
`web/shell.js`, which closed the *understated spec constant* — but the home is
single in the **tree**, not in what the page runs: the spec imports
`../../web/shell.js` and the page loads `harness/serve/app/shell.js`, the copy
`web/assemble.sh` put there. A stale `serve/` restores the exact vacuity, and
review demonstrated it — shrink the constant in source only, revert both socket
arms, 6/6 green. `createShell` now exposes `timeoutMs()` beside `timesArmed()`
and `outlastTheWatchdog` reads the running page's value and requires it to equal
the import. Injected with the ticket's own recipe (`8000` → `100` in
`web/shell.js`, no rebuild): the run goes to 5 passed / 1 failed, on
"the page's handshake window is not the one this spec imported", where the same
staleness was previously invisible.
Note what this does and does not reach: it guards the tests that *call the
helper*. `"a peer that accepts and then says nothing trips the watchdog"` also
names `HANDSHAKE_TIMEOUT_MS`, but as a Playwright `timeout:` margin rather than
as a window to outlast — a stale value there costs patience, not evidence.
**The window is single-homed, and that is the second half of the same **The window is single-homed, and that is the second half of the same
finding.** `HANDSHAKE_TIMEOUT_MS` lived twice — once in `web/app.js`, once in finding.** `HANDSHAKE_TIMEOUT_MS` lived twice — once in `web/app.js`, once in
the spec — with a comment saying a silent disagreement "would pass for the the spec — with a comment saying a silent disagreement "would pass for the
+32
View File
@@ -699,3 +699,35 @@ The one spelling `sp+` costs, stated rather than discovered later:
required its space since improvise-4zl7, so this is the two keywords agreeing. required its space since improvise-4zl7, so this is the two keywords agreeing.
The paired positive arm — the same text with the space — is what stops it The paired positive arm — the same text with the space — is what stops it
degrading into "any WHERE with a quoted category fails". degrading into "any WHERE with a quoted category fails".
### the_grammar_is_parsed_once_however_many_files_the_strategy_yields
improvise-8shu. `improvise-io`'s lib binary was the run's outlier — 1.42s here
against a 0.01s median, measured at 1.40–1.45s, 1.50s and 3.70–5.24s on three
machines. The ticket's diagnosis was that the grammar walk had no size bound,
so a few deeply nested documents dominated. **That was wrong**, and measuring
before fixing is the only reason it did not ship as the fix: instrumenting the
two halves separately gave `load_grammar()` ×1000 = **2.50s** and the walk
itself ×1000 = **15.7ms**. The walk was 0.6% of the cost. `improv_file()`
called `load_grammar()` inside its `prop_map`, so `pest_meta` re-parsed
`improv.pest` once for every generated case — 1000 times across the two
properties, plus shrink attempts.
The strategy's input space was never the problem and is unchanged: the same
entropy range, the same 0..N repetitions, the same properties over groups,
formulas, views, hidden items and pipe quoting. The grammar is now read once
through a `OnceLock`.
The guard counts *parses* rather than calls, which is what makes it
order-independent: other tests in the binary reach `grammar()` too, and the
count stays 1 however many of them run first. It drives `improv_file()` itself
through a `TestRunner` rather than re-implementing what the strategy does, so
a future edit that reverts the call site is what it catches. Fault injected:
`Gen::new(grammar(), …)` restored to `let rules = load_grammar();` — the test
fails with `left: 64, right: 1`.
Case counts went 500 → 256 in the same file, which is the default
`context/design-principles.md` §7 prescribes and never below it; that is worth
a further 0.39s → 0.21s but it is not the fix, and reporting it as one would
be the mistake this entry exists to record. Net: 1.42s → 0.21s, and
`improvise-io` is no longer the run's longest binary.
+36
View File
@@ -577,3 +577,39 @@ value is written out in the test name or the assertion.
The third assertion is what stops this one being satisfiable by accident: 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 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. 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` is **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, so it is a filed gap (improvise-zv8t) rather than a refusal. 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.
+166 -20
View File
@@ -24,9 +24,55 @@ pub enum Spelling {
Measure(String), Measure(String),
/// `Cat IS Item` — a coordinate predicate; the document must have `Cat`. /// `Cat IS Item` — a coordinate predicate; the document must have `Cat`.
Coordinate { category: String, item: String }, Coordinate { category: String, item: String },
/// An encoding artefact: a `col.missing` flag or a date part, which the /// An encoding artefact the formula language cannot spell at all. The arm
/// formula language cannot spell at all. /// names *which* family, because they are declined for different reasons.
Artefact, Artefact(Artefact),
}
/// Why a feature is unspellable. Five families, declined separately — each needs
/// different vocabulary to close. `context/code/gp.md` §11 carries the arguments.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum Artefact {
/// `col.missing`: "this record has no value for that column" — of a measure
/// or of a category, so the words must not claim either (improvise-wd6n).
MissingFlag,
/// `when.year` and its siblings: date arithmetic (improvise-wd6n).
DatePart,
/// A one-hot over a *measure* column — the record's value, not a coordinate.
ValueOfAMeasure,
/// The `(missing)` pseudo-item: the absence of a coordinate, not an item.
AbsentCoordinate,
/// A category column whose items all parse as numbers; a reference to its
/// name would resolve to a `_Measure` item, which is a different column.
NumericCategory,
}
impl Artefact {
/// The refusal's own clause — read off a status bar, so one line each.
fn reason(self) -> &'static str {
match self {
Self::MissingFlag => {
"says this record has no value for that column, which the formula language \
has no word for: a reference to an absent cell is an error, not a zero"
}
Self::DatePart => {
"is a part of a date, and date arithmetic is something improvise does not have \
anywhere: a cell holding a date is text, and nothing in eval reads one"
}
Self::ValueOfAMeasure => {
"is a value of a measure rather than a coordinate, so `IS` cannot ask about it \
— a measure's values are text, and its absent case is no value at all"
}
Self::AbsentCoordinate => {
"is the absence of a coordinate rather than an item, which `IS` reads as false \
rather than as a question"
}
Self::NumericCategory => {
"names a category whose items all parse as numbers, so a reference to it would \
resolve to a `_Measure` item of the same name — a different column"
}
}
}
} }
/// One fitted feature: what `preprocess` called it, and how improvise says it. /// One fitted feature: what `preprocess` called it, and how improvise says it.
@@ -84,7 +130,13 @@ fn spell(role: &FeatureRole, columns: &[ColumnSpec], categories: &[String]) -> S
item: value.clone(), item: value.clone(),
} }
} }
_ => Spelling::Artefact, // No wildcard: a new `FeatureRole` must be classified rather than
// silently declining as whichever family the message happened to name.
FeatureRole::Numeric { .. } => Spelling::Artefact(Artefact::NumericCategory),
FeatureRole::DatePart { .. } => Spelling::Artefact(Artefact::DatePart),
FeatureRole::Missing { .. } => Spelling::Artefact(Artefact::MissingFlag),
FeatureRole::OneHot { .. } if !is_category => Spelling::Artefact(Artefact::ValueOfAMeasure),
FeatureRole::OneHot { .. } => Spelling::Artefact(Artefact::AbsentCoordinate),
} }
} }
@@ -98,6 +150,31 @@ pub fn formula_text(tree: &Tree, features: &Features, target: &str) -> String {
) )
} }
/// The refused features grouped by family, each family's names in tree order.
/// Grouping is what lets the refusal give one reason per family, not per feature.
fn artefacts_by_family<'a>(used: &[&'a Feature]) -> Vec<(Artefact, Vec<&'a str>)> {
let mut families: Vec<(Artefact, Vec<&str>)> = Vec::new();
for feature in used {
let Spelling::Artefact(family) = feature.spelling else {
continue;
};
match families.iter_mut().find(|(f, _)| *f == family) {
Some((_, names)) => names.push(feature.name.as_str()),
None => families.push((family, vec![feature.name.as_str()])),
}
}
families
}
/// One clause per family: the names it refused, then that family's own reason.
fn artefact_clauses(families: &[(Artefact, Vec<&str>)]) -> String {
families
.iter()
.map(|(family, names)| format!("{names:?} {}", family.reason()))
.collect::<Vec<_>>()
.join("; and ")
}
/// [`formula_text`], parsed **and** checked against the document it is to be stored /// [`formula_text`], parsed **and** checked against the document it is to be stored
/// in — the form improvise stores. Why, and what is left: `context/code/gp.md` §11. /// in — the form improvise stores. Why, and what is left: `context/code/gp.md` §11.
pub fn fitted_formula( pub fn fitted_formula(
@@ -108,18 +185,14 @@ pub fn fitted_formula(
model: &Model, model: &Model,
) -> anyhow::Result<Formula> { ) -> anyhow::Result<Formula> {
let used = used_features(tree, features); let used = used_features(tree, features);
let artefacts: Vec<&str> = used let artefacts = artefacts_by_family(&used);
.iter()
.filter(|f| f.spelling == Spelling::Artefact)
.map(|f| f.name.as_str())
.collect();
anyhow::ensure!( anyhow::ensure!(
artefacts.is_empty(), artefacts.is_empty(),
// One line, because it is read off a status bar: a `rustfmt` pass over a // One line, because it is read off a status bar: a `rustfmt` pass over a
// multi-line literal leaves runs of ten spaces mid-sentence. // multi-line literal leaves runs of ten spaces mid-sentence.
"the fitted tree splits on {artefacts:?}, which the formula language cannot spell \ "the fitted tree splits on features the formula language cannot spell at all, so \
at all — a `col.missing` flag or a date part is an encoding artefact rather than \ nothing {target:?} could refer to: {}. Fit over measures and categories for now",
anything {target:?} could refer to. Fit over measures and categories for now", artefact_clauses(&artefacts),
); );
let unresolvable: Vec<&str> = used let unresolvable: Vec<&str> = used
.iter() .iter()
@@ -147,7 +220,7 @@ fn resolves_in(spelling: &Spelling, model: &Model) -> bool {
match spelling { match spelling {
Spelling::Measure(name) => model.find_item_category(name).is_some(), Spelling::Measure(name) => model.find_item_category(name).is_some(),
Spelling::Coordinate { category, .. } => model.category(category).is_some(), Spelling::Coordinate { category, .. } => model.category(category).is_some(),
Spelling::Artefact => false, Spelling::Artefact(_) => false,
} }
} }
@@ -203,7 +276,7 @@ fn spelling_text(spelling: &Spelling) -> String {
Spelling::Coordinate { category, item } => { Spelling::Coordinate { category, item } => {
format!("{} IS {}", quote_name(category), quote_name(item)) format!("{} IS {}", quote_name(category), quote_name(item))
} }
Spelling::Artefact => { Spelling::Artefact(_) => {
panic!("expr_text wants a feature the formula language can spell") panic!("expr_text wants a feature the formula language can spell")
} }
} }
@@ -272,14 +345,15 @@ mod tests {
) )
} }
/// Features no formula can spell: a `col.missing` flag or a date part. /// Features no formula can spell. The family is named rather than assumed:
fn artefacts(names: &[&str]) -> Features { /// these spellings are fabricated, and `Features::of` is what really decides.
fn artefacts(family: Artefact, names: &[&str]) -> Features {
Features( Features(
names names
.iter() .iter()
.map(|n| Feature { .map(|n| Feature {
name: (*n).to_string(), name: (*n).to_string(),
spelling: Spelling::Artefact, spelling: Spelling::Artefact(family),
}) })
.collect(), .collect(),
) )
@@ -607,7 +681,7 @@ mod tests {
/// `improvise_eval`, which fabricates an item per feature name (improvise-7na). /// `improvise_eval`, which fabricates an item per feature name (improvise-7na).
mod evaluability_in_a_real_document { mod evaluability_in_a_real_document {
use super::{MEASURE, TARGET, artefacts, coordinates, leaf, measures, split}; use super::{MEASURE, TARGET, artefacts, coordinates, leaf, measures, split};
use crate::{fitted_formula, formula_text}; use crate::{Artefact, fitted_formula, formula_text};
use improvise_core::formula::parse_formula; use improvise_core::formula::parse_formula;
use improvise_core::model::cell::{CellKey, CellValue}; use improvise_core::model::cell::{CellKey, CellValue};
use improvise_core::model::types::Model; use improvise_core::model::types::Model;
@@ -723,9 +797,14 @@ mod tests {
/// enumerates them: the check asks the precondition `Expr::Ref` has. /// enumerates them: the check asks the precondition `Expr::Ref` has.
#[test] #[test]
fn the_missing_flag_and_the_date_sub_features_are_refused_too() { fn the_missing_flag_and_the_date_sub_features_are_refused_too() {
for name in ["Cost.missing", "when.year", "when.month"] { for (family, name) in [
(Artefact::MissingFlag, "Cost.missing"),
(Artefact::DatePart, "when.year"),
(Artefact::DatePart, "when.month"),
] {
let tree = split(0, 0.5, leaf(1.0), leaf(2.0)); let tree = split(0, 0.5, leaf(1.0), leaf(2.0));
let err = fitted_formula(&tree, &artefacts(&[name]), TARGET, MEASURE, &document()) let features = artefacts(family, &[name]);
let err = fitted_formula(&tree, &features, TARGET, MEASURE, &document())
.expect_err(name) .expect_err(name)
.to_string(); .to_string();
assert!(err.contains(name), "{err}"); assert!(err.contains(name), "{err}");
@@ -765,4 +844,71 @@ mod tests {
assert!(fitted_formula(&tree, &features, TARGET, MEASURE, &model).is_ok()); assert!(fitted_formula(&tree, &features, TARGET, MEASURE, &model).is_ok());
} }
} }
/// improvise-wd6n. The families a `preprocess` run reaches — three of five —
/// are refused separately. Evidence: `context/test-evidence/gp.md`.
mod the_artefact_families_this_fixture_reaches_are_refused_separately {
use super::{MEASURE, TARGET, leaf, split};
use crate::{Features, fitted_formula};
use gp_tree::data::{Overrides, preprocess};
use improvise_core::model::types::Model;
/// One document category (`Region`), a text-valued measure, a measure with a
/// hole in it, and a date column — one row per artefact family.
fn features() -> Features {
let header: Vec<String> = ["Region", "Grade", "Cost", "When", "Score"]
.iter()
.map(|s| s.to_string())
.collect();
let rows: Vec<Vec<String>> = [
["North", "A", "40", "2024-01-02", "1"],
["South", "B", "", "2024-06-03", "2"],
["North", "A", "70", "2024-09-04", "3"],
["South", "B", "10", "2024-11-05", "4"],
]
.iter()
.map(|r| r.iter().map(|s| s.to_string()).collect())
.collect();
let data = preprocess(&header, &rows, &Overrides::default());
Features::of(&data, &header[..1])
}
fn refusal_for(features: &Features, name: &str) -> String {
let feat = (0..features.len())
.find(|i| features.get(*i).name == name)
.unwrap_or_else(|| {
panic!(
"preprocess emitted no {name:?}; it emitted {:?}",
(0..features.len())
.map(|i| features.get(i).name.as_str())
.collect::<Vec<_>>()
)
});
let tree = split(feat, 0.5, leaf(1.0), leaf(2.0));
fitted_formula(&tree, features, TARGET, MEASURE, &Model::new("d"))
.expect_err(name)
.to_string()
}
/// Each family's own reason, and **none of the others'** — one message
/// naming every family at once would pass the first clause alone.
#[test]
fn a_missing_flag_a_date_part_and_a_measures_value_are_refused_differently() {
let features = features();
let missing = "has no value for";
let date = "date arithmetic";
let text = "a value of a measure";
let err = refusal_for(&features, "Cost.missing");
assert!(err.contains(missing), "{err}");
assert!(!err.contains(date) && !err.contains(text), "{err}");
let err = refusal_for(&features, "When.year");
assert!(err.contains(date), "{err}");
assert!(!err.contains(missing) && !err.contains(text), "{err}");
let err = refusal_for(&features, "Grade=A");
assert!(err.contains(text), "{err}");
assert!(!err.contains(missing) && !err.contains(date), "{err}");
}
}
} }
+35 -4
View File
@@ -633,7 +633,7 @@ mod parser_prop_tests {
} }
proptest! { proptest! {
#![proptest_config(ProptestConfig::with_cases(500))] #![proptest_config(ProptestConfig::with_cases(256))]
/// improvise-pczv as a property rather than a list of clauses. /// improvise-pczv as a property rather than a list of clauses.
/// Evidence: `context/test-evidence/formula-and-io.md`. /// Evidence: `context/test-evidence/formula-and-io.md`.
@@ -1323,9 +1323,23 @@ mod generator {
use pest_meta::parser; use pest_meta::parser;
use proptest::prelude::*; use proptest::prelude::*;
use std::collections::HashMap; use std::collections::HashMap;
use std::sync::OnceLock;
use std::sync::atomic::{AtomicUsize, Ordering};
/// Counts how often the grammar text is actually re-read; the guard below
/// is what keeps `grammar()` from becoming a per-case parse again.
static GRAMMAR_PARSES: AtomicUsize = AtomicUsize::new(0);
/// The parsed grammar, read once for the process. Evidence:
/// `context/test-evidence/formula-and-io.md`.
fn grammar() -> &'static HashMap<String, (RuleType, Expr)> {
static RULES: OnceLock<HashMap<String, (RuleType, Expr)>> = OnceLock::new();
RULES.get_or_init(load_grammar)
}
/// Parse the grammar file and return rules keyed by name. /// Parse the grammar file and return rules keyed by name.
fn load_grammar() -> HashMap<String, (RuleType, Expr)> { fn load_grammar() -> HashMap<String, (RuleType, Expr)> {
GRAMMAR_PARSES.fetch_add(1, Ordering::Relaxed);
let grammar = include_str!("improv.pest"); let grammar = include_str!("improv.pest");
let pairs = parser::parse(parser::Rule::grammar_rules, grammar) let pairs = parser::parse(parser::Rule::grammar_rules, grammar)
.unwrap_or_else(|e| panic!("Bad grammar: {e}")); .unwrap_or_else(|e| panic!("Bad grammar: {e}"));
@@ -1449,11 +1463,28 @@ mod generator {
pub fn improv_file() -> impl Strategy<Value = String> { pub fn improv_file() -> impl Strategy<Value = String> {
// Use random bytes as entropy for choices in the grammar walk // Use random bytes as entropy for choices in the grammar walk
prop::collection::vec(any::<u8>(), 64..=256).prop_map(|choices| { prop::collection::vec(any::<u8>(), 64..=256).prop_map(|choices| {
let rules = load_grammar(); let mut g = Gen::new(grammar(), choices);
let mut g = Gen::new(&rules, choices);
g.generate("file") g.generate("file")
}) })
} }
/// improvise-8shu. Evidence: `context/test-evidence/formula-and-io.md`.
#[test]
fn the_grammar_is_parsed_once_however_many_files_the_strategy_yields() {
use proptest::strategy::ValueTree;
use proptest::test_runner::TestRunner;
let mut runner = TestRunner::deterministic();
let strategy = improv_file();
for _ in 0..64 {
let _ = strategy.new_tree(&mut runner).unwrap().current();
}
assert_eq!(
GRAMMAR_PARSES.load(Ordering::Relaxed),
1,
"improv.pest was re-parsed per generated case"
);
}
} }
#[cfg(test)] #[cfg(test)]
@@ -1464,7 +1495,7 @@ mod grammar_prop_tests {
use proptest::prelude::*; use proptest::prelude::*;
proptest! { proptest! {
#![proptest_config(ProptestConfig::with_cases(500))] #![proptest_config(ProptestConfig::with_cases(256))]
/// parse(generate()) — every generated file parses without error. /// parse(generate()) — every generated file parses without error.
#[test] #[test]
+2
View File
@@ -214,6 +214,8 @@ test.describe("a pointer event reaches the client and its answer is painted", ()
// A formula row: navigation only, because FormulaPanel binds no Enter and // A formula row: navigation only, because FormulaPanel binds no Enter and
// a click mirrors Enter. The Rust guard is on the KEYMAP — clients.md. // a click mirrors Enter. The Rust guard is on the KEYMAP — clients.md.
// `#grid input` toHaveCount(0) belongs here and is deliberately absent:
// it cannot fail, measured by injection (improvise-qf57, clients.md).
await page.keyboard.press("Shift+F"); await page.keyboard.press("Shift+F");
await expect(page.locator("#formulas li").first()).toBeVisible(); await expect(page.locator("#formulas li").first()).toBeVisible();
await page.keyboard.press("Escape"); await page.keyboard.press("Escape");
+6
View File
@@ -41,6 +41,12 @@ async function outlastTheWatchdog(page) {
await page.evaluate(() => window.__improviseShell?.timesArmed() ?? 0), await page.evaluate(() => window.__improviseShell?.timesArmed() ?? 0),
"the watchdog was never armed on this path: denying its message proves nothing", "the watchdog was never armed on this path: denying its message proves nothing",
).toBeGreaterThan(0); ).toBeGreaterThan(0);
// The import is the SOURCE tree's constant; the page runs the ASSEMBLED
// copy. A stale `serve/` makes them differ — improvise-669b, clients.md.
expect(
await page.evaluate(() => window.__improviseShell?.timeoutMs() ?? null),
"the page's handshake window is not the one this spec imported: re-run harness/build.sh",
).toBe(HANDSHAKE_TIMEOUT_MS);
const jump = HANDSHAKE_TIMEOUT_MS + 2_000; const jump = HANDSHAKE_TIMEOUT_MS + 2_000;
const before = await page.evaluate(() => Date.now()); const before = await page.evaluate(() => Date.now());
await page.clock.runFor(jump); await page.clock.runFor(jump);
+3
View File
@@ -153,6 +153,9 @@ export function createShell({ onStatus, timeoutMs, setTimer = setTimeout, clearT
this.setShell(text); this.setShell(text);
}, },
armed: () => watchdog !== null, armed: () => watchdog !== null,
/** The window this shell was built with. Exposed so a spec can compare the
* RUNNING page's value against the one it imported — improvise-669b. */
timeoutMs: () => timeoutMs,
/** How many times the watchdog has been armed. A test denying its message /** How many times the watchdog has been armed. A test denying its message
* needs this to be nonzero, or it is denying an unreachable event. */ * needs this to be nonzero, or it is denying an unreachable event. */
timesArmed: () => arms, timesArmed: () => arms,