fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(gp): decline the artefact families separately, in their own words

Browse Source 
improvise-wd6n. Spelling::Artefact was one variant standing for five distinct
families, the last five reached through a `_ =>` wildcard in spell(). The one
refusal they 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.

Artefact now names which family, spell() is exhaustive (a sixth FeatureRole is
a compile error rather than a sixth thing that message misdescribes), and the
refusal gives one clause per family.

The two families the ticket asks about are declined in writing in
context/code/gp.md §11, separately, because each needs different vocabulary:
col.missing needs a word for absence that reaches apply_agg and eval_agg, and
the date parts need a date type improvise does not have anywhere. Nothing new
is emitted, so improvise-4zl7's route-then-aggregate rule is untouched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
Edward Langley
2026-09-20 21:51:17 -07:00
parent 97177112a3
commit 173d9820a6
3 changed files with 242 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files
context/code/gp.md
+47 -8
View File
@@ -289,7 +289,19 @@ never by the shape of the name:
|---|---|---|
| `Numeric` | a measure | `Expr::Ref(measure)` |
| `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
no coordinate at all — the record's value for that measure is text, and
@@ -326,14 +338,41 @@ flags stay covered without a second copy of `preprocess`'s naming rules.
`Artefact` reaching it is a precondition failure and panics, the same split
`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
vocabulary, and one the formula language has no word for at all. The date
sub-features (`when.year`, `when.month`) need date arithmetic improvise does not
have. Both stay `Artefact`, both stay refused, and both are filed as
improvise-wd6n rather than smuggled in — separately from each other, because
they need different vocabulary.
Both stay `Artefact` and both stay refused. The decline is written here rather
than left open, because each needs a **formula-language** change and neither
needs an emitter change — and they are answered separately because the
vocabulary each wants is different. What changed with the decline is that the
refusal now says which, in that family's own words.
**`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.
The other three families are declined permanently rather than pending: a one-hot
over a measure and the `(missing)` pseudo-item name no coordinate *by
construction*, and a numeric category column is a name collision the document
itself creates. No vocabulary closes those; they are refusals, not gaps.
## 12. A fit is persisted as its formula, and nothing else (improvise-7na)
context/test-evidence/gp.md
+30
View File
@@ -577,3 +577,33 @@ 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). The remaining two —
`AbsentCoordinate` and `NumericCategory` — are not reachable from it and are not
asserted here; they are refusals rather than pending gaps, argued in
`context/code/gp.md` §11.
crates/improvise-gp/src/lib.rs
+165 -20
View File
@@ -24,9 +24,54 @@ pub enum Spelling {
Measure(String),
/// `Cat IS Item` — a coordinate predicate; the document must have `Cat`.
Coordinate { category: String, item: String },
/// An encoding artefact: a `col.missing` flag or a date part, which the
/// formula language cannot spell at all.
Artefact,
/// An encoding artefact the formula language cannot spell at all. The arm
/// names *which* family, because they are declined for different reasons.
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 X" (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 measure, 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 \
— the record's value for that column is text"
}
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.
@@ -84,7 +129,13 @@ fn spell(role: &FeatureRole, columns: &[ColumnSpec], categories: &[String]) -> S
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 +149,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
/// in — the form improvise stores. Why, and what is left: `context/code/gp.md` §11.
pub fn fitted_formula(
@@ -108,18 +184,14 @@ pub fn fitted_formula(
model: &Model,
) -> anyhow::Result<Formula> {
let used = used_features(tree, features);
let artefacts: Vec<&str> = used
.iter()
.filter(|f| f.spelling == Spelling::Artefact)
.map(|f| f.name.as_str())
.collect();
let artefacts = artefacts_by_family(&used);
anyhow::ensure!(
artefacts.is_empty(),
// 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.
"the fitted tree splits on {artefacts:?}, which the formula language cannot spell \
at all — a `col.missing` flag or a date part is an encoding artefact rather than \
anything {target:?} could refer to. Fit over measures and categories for now",
"the fitted tree splits on features the formula language cannot spell at all, so \
nothing {target:?} could refer to: {}. Fit over measures and categories for now",
artefact_clauses(&artefacts),
);
let unresolvable: Vec<&str> = used
.iter()
@@ -147,7 +219,7 @@ fn resolves_in(spelling: &Spelling, model: &Model) -> bool {
match spelling {
Spelling::Measure(name) => model.find_item_category(name).is_some(),
Spelling::Coordinate { category, .. } => model.category(category).is_some(),
Spelling::Artefact => false,
Spelling::Artefact(_) => false,
}
}
@@ -203,7 +275,7 @@ fn spelling_text(spelling: &Spelling) -> String {
Spelling::Coordinate { category, item } => {
format!("{} IS {}", quote_name(category), quote_name(item))
}
Spelling::Artefact => {
Spelling::Artefact(_) => {
panic!("expr_text wants a feature the formula language can spell")
}
}
@@ -272,14 +344,15 @@ mod tests {
)
}
/// Features no formula can spell: a `col.missing` flag or a date part.
fn artefacts(names: &[&str]) -> Features {
/// Features no formula can spell. The family is named rather than assumed:
/// these spellings are fabricated, and `Features::of` is what really decides.
fn artefacts(family: Artefact, names: &[&str]) -> Features {
Features(
names
.iter()
.map(|n| Feature {
name: (*n).to_string(),
spelling: Spelling::Artefact,
spelling: Spelling::Artefact(family),
})
.collect(),
)
@@ -607,7 +680,7 @@ mod tests {
/// `improvise_eval`, which fabricates an item per feature name (improvise-7na).
mod evaluability_in_a_real_document {
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::model::cell::{CellKey, CellValue};
use improvise_core::model::types::Model;
@@ -723,9 +796,14 @@ mod tests {
/// enumerates them: the check asks the precondition `Expr::Ref` has.
#[test]
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 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)
.to_string();
assert!(err.contains(name), "{err}");
@@ -765,4 +843,71 @@ mod tests {
assert!(fitted_formula(&tree, &features, TARGET, MEASURE, &model).is_ok());
}
}
/// improvise-wd6n. The artefact families are refused *separately*, because
/// they are declined for different reasons. Evidence: `context/test-evidence/gp.md`.
mod each_artefact_family_is_refused_in_its_own_words {
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}");
}
}
}