Merge pull request #10: waves 2 and 3 — the projection producer, one reducer, one parser walker, one quoting home
CI / Test and lint (host) (push) Has been cancelled
CI / wasm32 check (browser bundle crates) (push) Has been cancelled
CI / Knowledge base consistency (push) Has been cancelled

Wave 2: ui/project.rs (improvise-cqq) decides what each client must be told by diffing against a per-client mirror rather than asking effects what they touched; ui/reduce.rs (improvise-gxi) is the one door from a Command or a keypress to the authoritative App, with its ACs 4-5 deliberately unmet and filed as improvise-kmgs; prune_axis_items takes a predicate so its length invariant is unrepresentable rather than asserted (improvise-n3wt).

Wave 3: parse_md is one module-scope walker per .improv section (improvise-heqk); pipe quoting has one home in improvise-formula::quote, with a user-visible format delta -- a category named SUM now writes as |SUM| (improvise-i2m5); gp-tree returns data and never prints, records its GA config beside the seed, and owns its GA loop (improvise-d44s/ksst/0ai).

Five bugs found by review, each proven by a failing test first: Dirty::of(LayoutGeometry) under-reported formulas and stranded deleted formulas on every client forever; ClientSession::apply accepted SwitchView and desynced the mirror; a document swap left every session mirror keyed to the old document; a view switch moved only the asking session s mirror (Copilot); and folds(n, 0) panicked while mse divided by a length it had not scored (Copilot).
This commit is contained in:
Ed Langley
2026-09-19 14:59:56 -07:00
committed by GitHub
20 changed files with 2853 additions and 665 deletions
+32 -22
View File
File diff suppressed because one or more lines are too long
+5
View File
@@ -43,6 +43,11 @@ jobs:
run: cargo check -p improvise-io --target wasm32-unknown-unknown
- name: improvise-command
run: cargo check -p improvise-command --target wasm32-unknown-unknown
# Not in the browser bundle yet — nothing depends on improvise-gp. Gated
# anyway because it is a candidate (fitting a tree in the browser) and
# staying wasm-clean is free now, expensive to retrofit (improvise-z5eg).
- name: improvise-gp
run: cargo check -p improvise-gp --target wasm32-unknown-unknown
kb:
name: Knowledge base consistency
+40 -16
View File
@@ -36,6 +36,9 @@ Architectural intent lives in `context/design-principles.md` — read that for t
| Change number display | `format.rs` → `format_f64` |
| Change CLI args | `main.rs` (clap) |
| Change import logic | `import/wizard.rs` → `ImportPipeline` |
| Change how a gp-tree fit searches | `gp-tree/src/fit.rs` → `FitConfig` / `fit` |
| Change gp-tree CSV preprocessing | `gp-tree/src/data.rs` → `preprocess` |
| Change how a name is quoted anywhere | `improvise-formula/src/quote.rs` — the one home, both halves |
| Change import-wizard keys/steps | `command/cmd/wizard.rs` (keys) + `import/wizard.rs` (steps) |
| Change frame layout | `draw.rs` → `draw()` |
| Change app state / modes | `ui/app.rs` → `App`, `AppMode` |
@@ -144,8 +147,10 @@ Region=West, Measure=Revenue = |pending| # pipe-quoted text
## Gotchas (read before editing)
1. **Records mode is wide format.** A row is one *coordinate tuple* — the cells sharing everything but `_Measure` — and each raw measure gets its own editable column, followed by a `+` column that creates a new measure (improvise-c5v). Consequences: two measures on one coordinate are ONE row; editing a category column moves **every** measure cell in the row, not just the one under the cursor; `_Dim` in the synthetic key carries the column **index**, not a label, because a category and a measure may share a name — resolve it with `GridLayout::record_col` into a `RecordCol`. Formula targets are not columns (nothing to type into them). Drill pending edits are keyed by `(row, RecordCol)`.
**`prune_axis_items` takes a predicate, not a flag vector.** `prune_empty` hands it `impl Fn(usize) -> bool` over the *data* index (group headers consume no index). The domain is derived from `items` itself, so there is no parallel length a caller could get wrong — the earlier `&[bool]` + `debug_assert` pair could disagree between debug and release if a third caller ever sized `keep` from something other than `row_count()`/`col_count()` (improvise-n3wt).
2. **Effects never touch `std::fs`.** Every document read or write goes through `App::storage: Box<dyn Storage>` (improvise-fuj). The host picks the backend: `App::new` supplies `PhysicalFs`, `App::with_storage` takes any other — a browser worker will supply OPFS (improvise-i34), and tests use `MemoryFs`. `improvise-io` keeps path-taking wrappers (`save`, `load`, `export_csv`, `parse_csv_with_warnings`) for the native CLI, each delegating to a `_to`/`_from`/`_bytes` variant that takes the backend or the bytes.
3. **Every effect declares an `EffectKind`, and there is no default.** `ViewOnly` never reaches other clients; `Model(CellData | LayoutGeometry | WholeWorkbook)` does (improvise-mae). A new effect that forgets to classify itself fails to compile — that is deliberate, since the failure mode is silent: a mis-declared `ViewOnly` edit would simply never reach connected clients. Watch two traps: the *View* lives inside `model_state.workbook`, so "touches model_state" does NOT mean "is a model change" (axis/page/format are `LayoutGeometry`, cursor and scroll are `ViewOnly` — see improvise-8uz); and `Save`/`SaveAs`/`MarkDirty` touch `model_state` for bookkeeping only and are `ViewOnly`. `AddCategory`/`RemoveCategory` are the third trap and go the other way: they look like data edits, but both go through `Workbook`, whose `on_category_added`/`on_category_removed` rewrite **every** view's axis map, so they are `LayoutGeometry`. The two "an effect declaring X actually changes X" tests cannot separate them — a category change moves model data *and* view config, so it clears either bar; `structural_effects_declare_their_kind` pins the declaration itself.
3. **Every effect declares an `EffectKind`, and there is no default.** `ViewOnly` never reaches other clients; `Model(CellData | LayoutGeometry | WholeWorkbook)` does (improvise-mae). A new effect that forgets to classify itself fails to compile — that is deliberate, since the failure mode is silent: a mis-declared `ViewOnly` edit would simply never reach connected clients. Watch two traps: the *View* lives inside `model_state.workbook`, so "touches model_state" does NOT mean "is a model change" (axis/page/format are `LayoutGeometry`, cursor and scroll are `ViewOnly` — see improvise-8uz); and `Save`/`SaveAs`/`MarkDirty` touch `model_state` for bookkeeping only and are `ViewOnly`. `AddCategory`/`RemoveCategory` are the third trap and go the other way: they look like data edits, but both go through `Workbook`, whose `on_category_added`/`on_category_removed` rewrite **every** view's axis map, so they are `LayoutGeometry`. The two "an effect declaring X actually changes X" tests cannot separate them — a category change moves model data *and* view config, so it clears either bar; `structural_effects_declare_their_kind` pins the declaration itself. Their reach is wider than the name suggests: `Model::remove_category` also drops every formula targeting the category, which is why `Dirty::of(LayoutGeometry)` reports `formulas: true` — see gotcha 35.
4. **Commands never mutate.** `&CmdContext` is read-only; return `Vec<Box<dyn Effect>>`. If you want to touch `&mut App`, add or extend an Effect.
5. **`CellKey` is always sorted.** Use the constructors; equality and hashing rely on canonical form.
6. **No `Model::add_item`.** With a `Workbook` in hand use the facade: `wb.add_item("Region", "East")?` / `add_item_in_group` / `remove_item` (improvise-tvt). With only a `Model`, go through the category: `m.category_mut("Region").unwrap().add_item("East")`.
@@ -167,7 +172,7 @@ Region=West, Measure=Revenue = |pending| # pipe-quoted text
22. **The import wizard's back path is a history stack, not a reverse table.** `advance` pushes the step it leaves onto `ImportWizard::history`; `previous` pops it, so back always retraces the path actually taken through the conditional transitions (improvise-907). Preconditions live in `validation_error()`, checked on the step where the user can still fix them — never deferred to `build_model`. `NameModel` is terminal for `advance`: Enter there calls `build()`, which parks the workbook in `built` on `Done`, and `AcceptWizardModel` takes it from there. Going back from `Done` discards `built`.
23. **`app.layout` goes stale mid-batch.** `apply_effects` rebuilds only *after* the whole batch, so an effect that reads the layout after an earlier effect changed the grid needs a `RebuildLayout` effect in front of it — the command decides the order (see `EnterEditAtCursorCmd`, `CommitAndAdvance`). Read the cursor's cell text via `App::display_value_at_cursor()`; don't build a `CmdContext` inside `apply` (it recomputes column widths).
24. **The cursor is per-client and keyed by view.** `ViewState::cursors: HashMap<String, ViewCursor>` holds `selected`/`row_offset`/`col_offset`, keyed by view name; an absent key means the origin. Reach it through `App::cursor()` (a `Copy` snapshot), `App::cursor_mut()`, and `App::reset_cursor()` — never off `View`, which no longer has the fields (improvise-8uz). Two consequences. **Changing the grid's shape must reset the cursor explicitly**: `View::transpose_axes`/`cycle_axis` used to do it themselves, and now `TransposeAxes`/`CycleAxis` call `app.reset_cursor()` — a new geometry effect that renumbers rows or columns has to do the same. **Replacing the document must clear the map**: `LoadModel` and `AcceptWizardModel` call `cursors.clear()`, because the incoming workbook's view names have nothing to do with the outgoing one's. That clearing replaces `Workbook::normalize_view_state`, which is gone — a `View` can no longer carry a stale offset.
24. **The cursor is per-client and keyed by view.** `ViewState::cursors: HashMap<String, ViewCursor>` holds `selected`/`row_offset`/`col_offset`, keyed by view name; an absent key means the origin. Reach it through `App::cursor()` (a `Copy` snapshot), `App::cursor_mut()`, and `App::reset_cursor()` — never off `View`, which no longer has the fields (improvise-8uz). Two consequences. **Changing the grid's shape must reset the cursor explicitly**: `View::transpose_axes`/`cycle_axis` used to do it themselves, and now `TransposeAxes`/`CycleAxis` call `app.reset_cursor()` — a new geometry effect that renumbers rows or columns has to do the same. **Replacing the document must clear the map**: `LoadModel` and `AcceptWizardModel` call `cursors.clear()`, because the incoming workbook's view names have nothing to do with the outgoing one's. That clearing replaces `Workbook::normalize_view_state`, which is gone — a `View` can no longer carry a stale offset. The same clearing owes the **session mirrors**: `install_workbook` bumps `App::document_generation`, and `ClientSession::project` reseeds any mirror seeded at an earlier one (improvise-cqq). An effect cannot reach `Sessions`, so the generation is the mechanism and `Sessions::rebase` is only the client-facing half.
25. **`ViewState` and its field types are `Serialize`/`Deserialize`.** It is the wire payload of the client Snapshot (improvise-q08), so `AppMode`, `MinibufferConfig`, `MinibufColor`, `ViewFrame`, `KeyCode` and `KeyModifiers` all derive both (improvise-a2x), and `improvise-command` depends on `serde` for it. The constraint this puts on new state: **no borrowed-`'static` fields anywhere ViewState can reach** — `Deserialize` cannot produce one, and the error surfaces on every enclosing variant rather than at the offending field. `view_state_survives_a_json_round_trip` guards it. `ViewState` and `Snapshot` also derive `Clone` and `PartialEq`, so the bootstrap message can be compared in a round-trip test.
@@ -177,7 +182,7 @@ Region=West, Measure=Revenue = |pending| # pipe-quoted text
28. **The browser backend is sync on purpose, with an async seam.** `OpfsFs` (`improvise-io/src/opfs.rs`, wasm32 only) implements the same synchronous `Storage` trait as `PhysicalFs`, because `FileSystemFileHandle.createSyncAccessHandle()` yields a handle whose read/write/truncate/flush **are** blocking — available only inside a dedicated Web Worker, which is where improvise-9x6 puts the app. So `Storage` never became async. What is still async splits out into four explicit methods the worker awaits from its message loop: `open()`, `attach(path)` (must precede any read/write of that document — acquiring a handle is async even though using it is not), `refresh()` (repopulates the index `list` answers from), and `commit_deletes()`. Consequence: **`delete` is visible immediately but lands in OPFS at the end of the batch** — it drops the name from the index and queues a `removeEntry`. Handles are exclusive per file, so `attach` closes any handle it replaces and `detach` releases the lock. `storage_contract` in `storage.rs` is the behaviour every backend owes; it runs against both host backends and is the written-down spec `OpfsFs` must satisfy in a browser, where the harness cannot reach.
29. **"Which view is active" is stored twice, and the copies must move together.** `Workbook::active_view` is the document's; `ViewState::active_view` is the client's, and it is what keys `ViewState::cursors`. The second copy exists because `reduce_view` runs on a client that holds no `Workbook` (improvise-cqi). Only two writers are allowed to touch either — `App::switch_to_view` and `App::install_workbook` — and `the_two_active_view_names_never_diverge` guards it. Calling `workbook.switch_view()` directly desyncs them and the cursor map is then read under the wrong key; that is exactly how the 8uz characterization test broke during the extraction. Whether the two are *allowed* to diverge — two clients on different views — is improvise-g0f, deliberately unanswered here.
29. **"Which view is active" is stored twice, and the copies must move together.** `Workbook::active_view` is the document's; `ViewState::active_view` is the client's, and it is what keys `ViewState::cursors`. The second copy exists because `reduce_view` runs on a client that holds no `Workbook` (improvise-cqi). Only two writers are allowed to touch either — `App::switch_to_view` and `App::install_workbook` — and `the_two_active_view_names_never_diverge` guards it. Calling `workbook.switch_view()` directly desyncs them and the cursor map is then read under the wrong key; that is exactly how the 8uz characterization test broke during the extraction. Whether the two are *allowed* to diverge — two clients on different views — is improvise-g0f, and the answer is **yes, eventually**: the model is shared, the UI is not, so `Workbook::active_view` becomes the document's *default* (what a fresh client opens) and `ViewState::active_view` is authoritative per client. improvise-jytm owns the split; a "follow mode" binding two UIs together is deliberately not ruled out. Until then `Sessions::apply` keeps them in lockstep by routing `SwitchView` through `App::switch_to_view` — see gotcha 36.
30. **Two tests gate `ViewState` membership.** `ViewState` is the *portable* session slice — the one thing serialized into the Snapshot a newly-connected client gets (improvise-q08). A field may live there only if it is **portable** (plain data from the allowlisted crates, no `Rc`/`Arc`) and **snapshot-safe** (a fresh client inheriting the value is correct). `wizard` fails both — `ImportWizard` is an `improvise-io` type, and a half-open dialog is not something to inherit — and `drill_state` fails both — it holds an `Rc`, and `pending_edits` are uncommitted writes belonging to whoever staged them. Both live on `App` under the **session-local** residue tag (improvise-h0b). The `ViewState: Send` static assertion in `app.rs`'s tests is the compile-time proxy for portability: `Rc` is not `Send`, so a re-added shared pointer fails the build (the `Deserialize` derive from gotcha 25 now rejects it too, serde's `rc` feature being off). `Send` does *not* catch the origin-crate or snapshot-safety halves — check those by hand.
@@ -185,10 +190,16 @@ Region=West, Measure=Revenue = |pending| # pipe-quoted text
32. **There is no aggregate render cache — each widget folds its own stream.** `improvise-protocol::render` holds one cache type per widget (`GridCache`, `CategoryTreeCache`, `FormulaCache`, `ViewListCache`, `TileBarCache`), each implementing `WidgetCache` with its own `Update` type, and a `Projection` is one such update addressed to one widget (improvise-rkeq). `reduce_view` folds only `ViewState` now — it no longer takes a cache, and `Command::Project` passes through it untouched. Two consequences. **The overlap between caches is deliberate**: the tile bar and the category panel both carry `(category, axis)` and are updated by separate projections, so neither is woken by the other's changes; do not "fix" it by sharing a variant. **Bootstrap is a *compaction*, not a replay**: `Snapshot { view, seed: Vec<Projection> }` seeds a new client with a batch of the same projections the steady stream carries, so there is no whole-state type that could disagree with the incremental one. The seed is the server's *current* state, not the head of the history — `GridCache::seed()` builds it, and the property that earns it is `GridCache::folded(&cache.seed()) == *cache`. Nothing is defined for the help overlay (its content is static and `help_page` is already in `ViewState`) or for the import wizard (its projection shape waits on improvise-764 deciding where the wizard state lives).
33. **The grid's labels frame its cells, and `apply` enforces it — the fold is not commutative.** Three consequences in `improvise-protocol/src/render.rs`. (a) A `GridUpdate::Cells` that overtakes the `RowLabels`/`ColLabels` naming its positions is **dropped**, because `fit_to_labels` truncates to the labels it has; a client must not reorder the stream, and `a_cell_update_that_overtakes_its_labels_is_lost` pins it. (b) A label change that *renames* a position blanks the cell under it: `fit_to_labels` preserves by POSITION, which is only right where the label did not change, so `apply` calls `invalidate_relabelled` itself rather than trusting a producer to send `GridUpdate::Invalidated` first. `Invalidated` therefore now means only "contents stale, frame unchanged". (c) `col_widths` is re-fitted with the columns — a new column is drawn at its label's own character count until `ColWidths` catches up — because a renderer zips widths with labels and a length mismatch indexes off the end. There is still **no producer** in the tree; this is the shape `from_app` must meet. Note the fold tests: asserting `fold(prefix)+fold(rest) == fold(whole)` is vacuous (it is the definition of a left fold), so the tests that earn their place are the order-sensitivity one and the compaction-equivalence one — see `doctrine(fold_tests_must_test_order_not_the_axiom)`.
33. **The grid's labels frame its cells, and `apply` enforces it — the fold is not commutative.** Three consequences in `improvise-protocol/src/render.rs`. (a) A `GridUpdate::Cells` that overtakes the `RowLabels`/`ColLabels` naming its positions is **dropped**, because `fit_to_labels` truncates to the labels it has; a client must not reorder the stream, and `a_cell_update_that_overtakes_its_labels_is_lost` pins it. (b) A label change that *renames* a position blanks the cell under it: `fit_to_labels` preserves by POSITION, which is only right where the label did not change, so `apply` calls `invalidate_relabelled` itself rather than trusting a producer to send `GridUpdate::Invalidated` first. `Invalidated` therefore now means only "contents stale, frame unchanged". (c) **every arm that touches labels *or* widths re-fits** — `col_widths` is re-fitted with the columns — a new column is drawn at its label's own character count until `ColWidths` catches up — because a renderer zips widths with labels and a length mismatch indexes off the end. Fitting on the label arms alone is what let a late `ColWidths` put the mismatch straight back — a short update keeps each unheard-of column at its label's width, and a long one is dropped as having overtaken the frame that names it, exactly as (a) holds `Cells`. There is still **no producer** in the tree; this is the shape `from_app` must meet. Note the fold tests: asserting `fold(prefix)+fold(rest) == fold(whole)` is vacuous (it is the definition of a left fold), so the tests that earn their place are the order-sensitivity one and the compaction-equivalence one — see `doctrine(fold_tests_must_test_order_not_the_axiom)`.
34. **A formula target is unquoted, and pipe quoting has one home.** `parse_formula` strips the pipes off a target exactly as it does off an `Expr::Ref` — the target names a `_Measure` item, so a target that kept its delimiters made `add_formula` create an item literally named `|Total Revenue|`: a ghost column the user's real item never matched, and `Expr::Ref("Total Revenue")` never resolved to. Both the TUI formula minibuffer (`AddFormula`) and `.improv` load go through that one call, so both were affected. The writer emits `Formula::raw`, not `target`, so file text is unaffected either way — which is exactly why the round-trip tests never saw it, and why the fix repairs existing files on next load with no migration. The writing side of the convention is `improvise_formula::quote::quote_name`; `improvise-gp` re-exports it, and `improvise-io/src/persistence/quoting.rs` still carries a third copy whose keyword guard is absent. The guard is the canonical behaviour: bare keywords do parse today, but only because `agg_call` needs a `(` and PEG backtracking falls through to `ref_expr` — rule ordering, not a stated rule.
35. **`Dirty` over-reports on purpose, and its doc comment is the derivation.** `Dirty::of` maps an `EffectKind` to the widgets that kind can reach, and the two directions do not cost the same: over-reporting pays one redundant `Projection`, under-reporting leaves a client's cache wrong **forever**, with nothing in the stream to correct it. `LayoutGeometry` therefore includes `formulas`, because `RemoveCategory` wears it and `Model::remove_category` ends by dropping every formula targeting the category — a client's formula panel otherwise kept showing a formula the document no longer had. It now coincides with `Dirty::EVERYTHING` and is still spelled out, because the two are derived from different effects and need not stay equal. The doc comment on `Dirty::of` is the **only** record of that derivation, and it went stale once already — claiming `AddCategory` was `CellData` after it was reclassified — so reclassifying an effect means re-reading it.
36. **A client's command has one door, and the door takes the `App`.** `Sessions::apply(app, id, cmd)` is the only way a connected client's `Command` reaches the server; `ClientSession::fold` and the mirror's `ViewState` are private, so a transport cannot reach the mirror at all. The door takes the authoritative `App` because some commands have a **document half**: `SwitchView` routes through `apply_command`/`App::switch_to_view` (gotcha 29), and the mirror then takes what the document *accepted*, not what was asked — so a switch to a view that does not exist leaves the mirror put. Everything else is that client's alone and never touches the `App`'s own view slice: **the model is shared, the UI is not.** Folding `SwitchView` into the mirror alone was the bug — `viewport()` then read the cursor under the new view's key and the next `project()` built that client a grid out of the *other* view.
37. **`gp-tree` returns data and never prints, and a fit takes two things to reproduce.** Two rules with one root — the crate is a library a TUI, a wasm worker or a CLI may each host. (a) A decision the user should hear about comes back as **data**: `Dataset::warnings` carries the columns `preprocess` dropped for being entirely missing (improvise-d44s), and `fit.rs` hands back pooled `(prediction, actual)` pairs and a score per feature rather than tables. `examples/fit_csv.rs` owns every `println!`, and that is the only reason its output stayed byte-identical across these moves — adding an `eprintln!` to either module is the regression. (b) **The seed is necessary and not sufficient**: `data::Model` records `seed: Option<u64>` *and* `config: Option<FitConfig>`, because the same seed under a different population/generations/elite/penalty lands on a different tree (improvise-ksst). Both are `Option` for one reason — every `FitConfig` value and the seed `0` are legal, so nothing can double as "unknown". Note `#[serde(default)]` on an `Option` field is **documentation, not the mechanism**: serde defaults it anyway, so deleting the attribute fails no test, and a backward-compatibility test that only deletes it is not discriminating. Aim such a test at an absent field loading as the *default* instead of as unknown. The GA loop itself is `fit::fit` (improvise-0ai); `breed` remains one fitness-agnostic generation for callers whose fitness is not "predict this column".
---
## File Inventory
@@ -201,7 +212,7 @@ model/types.rs 2062 / 70t Model, formula eval (fixed-point), CRUD
model/cell.rs 650 / 28t CellKey (sorted), CellValue, DataStore (interned + index)
model/category.rs 222 / 6t Category, Item, Group, CategoryKind
model/symbol.rs 79 / 3t Symbol interning
view/layout.rs 1151 / 27t GridLayout, drill, records mode
view/layout.rs 1194 / 29t GridLayout, drill, records mode
view/types.rs 627 / 33t View config (axes, pages, hidden, collapsed, format)
view/axis.rs 205 / 5t Axis enum + label/color/next methods, AxisColor
workbook.rs 345 / 17t Workbook: Model + cross-view ops, item facade
@@ -252,16 +263,24 @@ key.rs 101 / 3t KeyCode, KeyModifiers, KeyEvent (moved from
### `gp-tree` / `improvise-gp` (`crates/gp-tree/`, `crates/improvise-gp/`)
```
gp-tree/src/lib.rs 626 / 6t Tree, Spec, eval, simplify, render, breed, Population,
SplitMix64 (the vendored seeded Rng; `seed()` keeps the
seed so a fit can be shown and recorded, improvise-lzn)
gp-tree/src/data.rs 534 / 12t raw string rows → f32 feature vectors: per-column type
inference, one-hot, mean-imputation + `col.missing`
flag, ColumnSpec/encode_row, Model (trees + plan)
gp-tree/examples/fit_csv.rs 787 CLI over the above: flags, GA loop, CV, reports
improvise-gp/src/lib.rs ~330 / 7t formula_text / fitted_formula: a Tree as `IF(...)` formula
text. Thresholds are emitted as the f32's *exact* decimal
so a row on a split boundary routes the same in f64.
gp-tree/src/lib.rs 669 / 8t Tree, Spec, eval, simplify, render, breed, Population,
SplitMix64 (vendored seeded Rng; seed() keeps the seed so
a fit can be shown and recorded, pinned to the published
reference vectors)
gp-tree/src/data.rs 900 / 19t raw string rows -> f32 feature vectors: per-column type
inference, one-hot, mean-imputation + col.missing flag,
ColumnSpec/encode_row, Dataset (incl. warnings),
Model = trees + plan + seed + FitConfig
gp-tree/src/fit.rs 425 / 7t the search: FitConfig (Default = the CLI's numbers), fit()
-> simplified champion, cv_regress/cv_classify,
permutation_importance, folds/standardize/mse.
Returns data; never prints
gp-tree/examples/fit_csv.rs 598 CLI over the above: flags, demo dataset, all printing
improvise-gp/src/lib.rs 485 / 14t formula_text / fitted_formula: a Tree as IF(...) formula
text, validated back through parse_formula. A threshold is
the SHORTEST decimal that reads back as the same f64 (not
the exact expansion), so a row on a split boundary routes
the same in f64 as in f32 eval; non-finite values panic
```
### Command layer (`crates/improvise-command/src/command/`)
@@ -286,7 +305,12 @@ parse.rs 236 / 19t Script/command-line parser (prefix syntax)
### Semantic UI state (`crates/improvise-command/src/ui/`)
```
ui/effect.rs 2644 / 63t Effect trait, 50+ effect types
ui/app.rs 2134 / 71t App, ModelState, ViewState, AppMode (16) + dispatch methods, Panel, handle_key, autosave, HELP_PAGE_COUNT
ui/app.rs 2176 / 71t App, ModelState, ViewState, AppMode (16) + dispatch methods, Panel, handle_key, dispatch_key, document_generation, autosave, HELP_PAGE_COUNT
ui/project.rs 1184 / 14t Dirty, Viewport, grid_of/category_rows/formula_texts/
view_summaries/tile_entries, grid_updates (the diff),
ClientSession + Sessions (connect/apply/rebase/project_all)
ui/reduce.rs 231 / 6t apply_command / reduce_full / reduce_key — the one door
from a Command or a keypress to the authoritative App
ui/cat_tree.rs 156 / 6t Category tree flattener for panel
ui/viewport.rs 128 compute_col_widths / row_header_width / visible_cols (pure geometry)
```
+35 -231
View File
@@ -31,7 +31,10 @@
//! `--seeds <n>` (keep the best of `n` GA restarts; default 1), `--penalty <λ>`
//! (per-node parsimony cost keeping trees small/general; default 0.01),
//! `--seed <n>` (the RNG seed; printed by every fit and saved with `--save`,
//! so a fit can be reproduced or a different search asked for).
//! so a fit can be reproduced or a different search asked for). `--save` also
//! records the `FitConfig` those flags produced, because the seed alone does
//! not reproduce a fit — the same seed under different knobs searches
//! differently.
//!
//! Real CSVs are messy, so: **missing cells** (blank or `NA`/`null`/`?`/…) are
//! mean-imputed for numeric/date columns — with a `col.missing` 0/1 indicator
@@ -66,20 +69,24 @@
//! fitness (here, negative MSE over the rows); the crate evolves, simplifies,
//! and renders the trees.
//!
//! The preprocessing described above is **not** in this file: type inference,
//! one-hot encoding, imputation, [`encode_row`] and [`Model`] all live in
//! [`gp_tree::data`], where any consumer can reach them. What is left here is
//! the CLI — flag parsing, the demo dataset, the fitness and GA loop, the
//! cross-validation and the reports.
//! Almost none of the above is **in** this file. Preprocessing — type
//! inference, one-hot encoding, imputation, [`encode_row`] and [`Model`] —
//! lives in [`gp_tree::data`]; the search — the GA loop, the parsimony
//! objective, `k`-fold cross-validation and permutation importance — lives in
//! [`gp_tree::fit`]. Both are reachable by any consumer. What is left here is
//! the CLI: flag parsing, the demo dataset, and the printing. The library
//! returns pooled predictions and a score per feature; deciding they become
//! these tables, on stdout, is this file's job and only this file's.
use gp_tree::data::{
Dataset, Forced, Model, Overrides, Target, TargetModel, encode_row, is_missing, preprocess,
read_csv,
};
use gp_tree::{
GaConfig, Population, Rng, Spec, SplitMix64, Tree, breed, eval, is_valid, random_tree, render,
simplify, size,
use gp_tree::fit::{
FitConfig, argmax, cv_classify, cv_regress, fit, misclass_rate, permutation_importance,
standardize,
};
use gp_tree::{Spec, SplitMix64, Tree, eval, is_valid, render, size};
/// The seed a fit uses when `--seed` is not given. Fixed rather than
/// clock-derived: two runs over the same CSV should agree by default, and a
@@ -118,93 +125,6 @@ fn demo_dataset() -> (Vec<String>, Vec<Vec<String>>) {
(header, rows)
}
/// Mean-squared error of a tree over the rows.
fn mse(t: &Tree, x: &[Vec<f32>], y: &[f32]) -> f32 {
let sse: f32 = x
.iter()
.zip(y)
.map(|(f, &yi)| {
let e = eval(t, f) - yi;
e * e
})
.sum();
sse / x.len() as f32
}
/// Knobs that shape the search: a parsimony penalty (bigger trees are worse, so
/// the fit stays small and general) and the number of independent GA restarts
/// to keep the best of (GP is stochastic — one run can land in a poor basin).
#[derive(Clone, Copy)]
struct Trainer {
penalty: f32,
seeds: usize,
}
impl Trainer {
/// Objective the GA minimises: MSE plus a per-node parsimony cost.
fn cost(&self, t: &Tree, x: &[Vec<f32>], y: &[f32]) -> f32 {
mse(t, x, y) + self.penalty * size(t) as f32
}
/// Evolve one tree minimising [`Trainer::cost`], returning the best
/// (simplified) tree across `seeds` independent restarts.
fn fit(&self, x: &[Vec<f32>], y: &[f32], spec: &Spec, rng: &mut impl Rng) -> Tree {
const POP: usize = 80;
const GENS: usize = 600;
let cfg = GaConfig {
pop: POP,
elite: 8,
immigrants: 8,
max_nodes: 25,
};
let mut best: Option<(f32, Tree)> = None;
for _ in 0..self.seeds.max(1) {
let mut pop = Population::seed(&random_tree(spec, 4, rng), POP, spec, rng);
let mut run_best = pop.members[0].clone();
let mut run_cost = self.cost(&run_best, x, y);
for _ in 0..GENS {
let mut scored: Vec<(f32, Tree)> = pop
.members
.iter()
.map(|t| (self.cost(t, x, y), t.clone()))
.collect();
scored.sort_by(|a, b| a.0.partial_cmp(&b.0).unwrap_or(std::cmp::Ordering::Equal));
if scored[0].0 < run_cost {
run_cost = scored[0].0;
run_best = scored[0].1.clone();
}
let best_first: Vec<Tree> = scored.into_iter().map(|(_, t)| t).collect();
pop.members = breed(best_first, &cfg, spec, rng);
}
let simplified = simplify(&run_best, spec);
let final_cost = self.cost(&simplified, x, y);
if best.as_ref().is_none_or(|(c, _)| final_cost < *c) {
best = Some((final_cost, simplified));
}
}
best.unwrap().1
}
}
/// The class whose tree scores highest for this row (one-vs-rest prediction).
fn argmax(trees: &[Tree], row: &[f32]) -> usize {
(0..trees.len())
.max_by(|&a, &b| {
eval(&trees[a], row)
.partial_cmp(&eval(&trees[b], row))
.unwrap_or(std::cmp::Ordering::Equal)
})
.unwrap()
}
/// Fraction of rows the one-vs-rest forest misclassifies.
fn misclass_rate(trees: &[Tree], x: &[Vec<f32>], codes: &[usize]) -> f32 {
let wrong = (0..x.len())
.filter(|&i| argmax(trees, &x[i]) != codes[i])
.count();
wrong as f32 / x.len() as f32
}
/// The confidence of a one-vs-rest prediction: softmax over the class scores,
/// taken at the winning class (1.0 = one class dominates, ~1/K = a toss-up).
fn confidence(scores: &[f32]) -> f32 {
@@ -214,43 +134,6 @@ fn confidence(scores: &[f32]) -> f32 {
exps.iter().copied().fold(f32::NEG_INFINITY, f32::max) / sum
}
/// Model-agnostic **permutation importance**: how much the model's error grows
/// when each feature column is shuffled across rows (severing its link to the
/// target), averaged over `repeats`. It calls only the black-box `err` closure,
/// so the same routine serves the regression tree and the classification forest
/// — no access to tree internals needed.
fn permutation_importance(
err: impl Fn(&[Vec<f32>]) -> f32,
x: &[Vec<f32>],
n_feat: usize,
repeats: usize,
rng: &mut impl Rng,
) -> Vec<f32> {
let base = err(x);
let n = x.len();
let mut work = x.to_vec();
let mut imp = vec![0.0; n_feat];
for (j, slot) in imp.iter_mut().enumerate() {
let col: Vec<f32> = x.iter().map(|r| r[j]).collect();
let mut acc = 0.0;
for _ in 0..repeats.max(1) {
let mut idx: Vec<usize> = (0..n).collect();
for i in (1..n).rev() {
idx.swap(i, rng.below(i + 1));
}
for i in 0..n {
work[i][j] = col[idx[i]];
}
acc += err(&work) - base;
}
for (i, &c) in col.iter().enumerate() {
work[i][j] = c; // restore the column for the next feature
}
*slot = (acc / repeats.max(1) as f32).max(0.0);
}
imp
}
/// Print the most important features (by permutation importance), as a share of
/// total importance with a little bar. Zero-importance features are omitted.
fn report_importance(names: &[String], imp: &[f32]) {
@@ -272,96 +155,6 @@ fn report_importance(names: &[String], imp: &[f32]) {
}
}
/// Standardize a target to ≈ zero-mean / unit-std, returning (normalized, mean,
/// std) — the crate's leaf scores live in a small range, so a large-magnitude
/// target has to be scaled in or it can't be reached (see the regression fit).
fn standardize(y: &[f32]) -> (Vec<f32>, f32, f32) {
let n = y.len() as f32;
let mean = y.iter().sum::<f32>() / n;
let std = (y.iter().map(|v| (v - mean).powi(2)).sum::<f32>() / n)
.sqrt()
.max(1e-6);
(y.iter().map(|v| (v - mean) / std).collect(), mean, std)
}
/// Split `0..n` into `k` folds after a deterministic shuffle, so held-out rows
/// are a representative sample rather than a contiguous block.
fn folds(n: usize, k: usize, rng: &mut impl Rng) -> Vec<Vec<usize>> {
let mut idx: Vec<usize> = (0..n).collect();
for i in (1..n).rev() {
idx.swap(i, rng.below(i + 1));
}
let mut out = vec![Vec::new(); k];
for (pos, &row) in idx.iter().enumerate() {
out[pos % k].push(row);
}
out
}
/// Sub-slice `x`/`y` to the given row indices.
fn take(x: &[Vec<f32>], y: &[f32], idx: &[usize]) -> (Vec<Vec<f32>>, Vec<f32>) {
(
idx.iter().map(|&i| x[i].clone()).collect(),
idx.iter().map(|&i| y[i]).collect(),
)
}
/// k-fold CV for regression: for each fold, standardize + fit on the other
/// folds, predict the held-out rows (de-normalized to original units), and
/// pool `(prediction, actual)` across all folds. Nothing here leaks the test
/// fold — the mean/std come from the training rows only.
fn cv_regress(
x: &[Vec<f32>],
y: &[f32],
spec: &Spec,
tr: Trainer,
k: usize,
rng: &mut impl Rng,
) -> Vec<(f32, f32)> {
let mut pooled = Vec::new();
for test in folds(y.len(), k, rng) {
let train: Vec<usize> = (0..y.len()).filter(|i| !test.contains(i)).collect();
let (tx, ty) = take(x, y, &train);
let (ty_norm, mean, std) = standardize(&ty);
let tree = tr.fit(&tx, &ty_norm, spec, rng);
for i in test {
pooled.push((eval(&tree, &x[i]) * std + mean, y[i]));
}
}
pooled
}
/// k-fold CV for classification: pool `(predicted_code, actual_code)` over the
/// held-out rows, fitting one-vs-rest trees on each training split.
fn cv_classify(
x: &[Vec<f32>],
codes: &[usize],
n_classes: usize,
spec: &Spec,
tr: Trainer,
k: usize,
rng: &mut impl Rng,
) -> Vec<(usize, usize)> {
let mut pooled = Vec::new();
for test in folds(codes.len(), k, rng) {
let train: Vec<usize> = (0..codes.len()).filter(|i| !test.contains(i)).collect();
let trees: Vec<Tree> = (0..n_classes)
.map(|c| {
let y: Vec<f32> = train
.iter()
.map(|&i| (codes[i] == c) as i32 as f32)
.collect();
let tx: Vec<Vec<f32>> = train.iter().map(|&i| x[i].clone()).collect();
tr.fit(&tx, &y, spec, rng)
})
.collect();
for i in test {
pooled.push((argmax(&trees, &x[i]), codes[i]));
}
}
pooled
}
/// Print pooled-CV regression metrics against the predict-the-mean baseline.
fn report_regression_cv(pooled: &[(f32, f32)]) {
let n = pooled.len() as f32;
@@ -447,10 +240,14 @@ struct FitOpts {
}
impl FitOpts {
fn trainer(&self) -> Trainer {
Trainer {
/// The CLI's flags on top of the library's default search. Only `penalty`
/// and `restarts` are exposed as flags; the rest of the GA's shape is
/// `FitConfig::default()`, which is where those numbers now live.
fn config(&self) -> FitConfig {
FitConfig {
penalty: self.penalty,
seeds: self.seeds,
restarts: self.seeds,
..FitConfig::default()
}
}
}
@@ -664,7 +461,13 @@ fn run_fit(csv_path: Option<String>, save_path: Option<String>, opts: FitOpts, o
rows: x,
target,
columns,
warnings,
} = preprocess(&header, &rows, ov);
// `preprocess` hands these back rather than printing them; deciding they go
// to stderr, in parentheses, is the CLI's call to make.
for warning in &warnings {
eprintln!("({warning})");
}
let name_refs: Vec<&str> = names.iter().map(String::as_str).collect();
let spec = Spec {
ranges: &ranges,
@@ -672,7 +475,7 @@ fn run_fit(csv_path: Option<String>, save_path: Option<String>, opts: FitOpts, o
};
let target_name = &header[header.len() - 1];
let mut rng = SplitMix64::new(opts.seed);
let tr = opts.trainer();
let cfg = opts.config();
// Fold count is clamped to the row count; CV is skipped for < 2 folds.
let k = opts.cv.min(rows.len());
@@ -683,7 +486,7 @@ fn run_fit(csv_path: Option<String>, save_path: Option<String>, opts: FitOpts, o
// *all* rows (so it's the strongest model to render/save), whose
// train MSE is optimistic by comparison.
if k >= 2 {
let pooled = cv_regress(&x, &y, &spec, tr, k, &mut rng);
let pooled = cv_regress(&x, &y, &spec, &cfg, k, &mut rng);
println!("=== regression: {target_name} ({k}-fold CV) ===");
report_regression_cv(&pooled);
println!();
@@ -696,7 +499,7 @@ fn run_fit(csv_path: Option<String>, save_path: Option<String>, opts: FitOpts, o
// The splits are scale-free; only the leaf values are affine-mapped
// back to real units, which we print alongside the tree.
let (y_norm, mean, std) = standardize(&y);
let tree = tr.fit(&x, &y_norm, &spec, &mut rng);
let tree = fit(&x, &y_norm, &spec, &cfg, &mut rng);
let train_mse = y
.iter()
.zip(&x)
@@ -733,7 +536,7 @@ fn run_fit(csv_path: Option<String>, save_path: Option<String>, opts: FitOpts, o
}
Target::Classify { labels, codes } => {
if k >= 2 {
let pooled = cv_classify(&x, &codes, labels.len(), &spec, tr, k, &mut rng);
let pooled = cv_classify(&x, &codes, labels.len(), &spec, &cfg, k, &mut rng);
println!(
"=== classification: {target_name} \u{2208} {{{}}} ({k}-fold CV) ===",
labels.join(", ")
@@ -746,7 +549,7 @@ fn run_fit(csv_path: Option<String>, save_path: Option<String>, opts: FitOpts, o
let trees: Vec<Tree> = (0..labels.len())
.map(|k| {
let y: Vec<f32> = codes.iter().map(|&c| (c == k) as i32 as f32).collect();
tr.fit(&x, &y, &spec, &mut rng)
fit(&x, &y, &spec, &cfg, &mut rng)
})
.collect();
let correct = (0..x.len())
@@ -783,6 +586,7 @@ fn run_fit(csv_path: Option<String>, save_path: Option<String>, opts: FitOpts, o
let model = Model {
target_name: target_name.clone(),
seed: Some(rng.seed()),
config: Some(cfg),
columns,
feature_names: names,
target: model,
+126 -7
View File
@@ -24,6 +24,9 @@
//! table may not even contain some of them. [`preprocess`] therefore records a
//! [`ColumnSpec`] per raw column, and [`encode_row`] replays it — that pairing
//! of fitted trees with their encoding plan is [`Model`].
//!
//! Anything [`preprocess`] decides that the user ought to hear about comes back
//! in [`Dataset::warnings`], never on stderr — see that field.
use serde::{Deserialize, Serialize};
@@ -99,13 +102,28 @@ pub enum TargetModel {
#[derive(Serialize, Deserialize)]
pub struct Model {
pub target_name: String,
/// The RNG seed the fit ran under, so the same fit can be re-run. `None`
/// for a model saved before the seed was recorded: `0` is a legal seed, so
/// a sentinel would make an unknown seed indistinguishable from a fit that
/// genuinely ran at 0, and anything reporting it would state a seed that
/// never ran.
/// The RNG seed the fit ran under. Necessary to re-run the fit, and **not
/// sufficient** on its own — the same seed under a different
/// [`FitConfig`] searches differently and lands somewhere else. The other
/// half is [`Model::config`]; the two are only meaningful together.
///
/// `None` for a model saved before the seed was recorded: `0` is a legal
/// seed, so a sentinel would make an unknown seed indistinguishable from a
/// fit that genuinely ran at 0, and anything reporting it would state a
/// seed that never ran.
#[serde(default)]
pub seed: Option<u64>,
/// The GA configuration the fit ran under — the other half of what it
/// takes to reproduce it. Recorded rather than left to the caller's memory
/// because [`Model::seed`]'s presence otherwise implies a completeness
/// that is not there: a saved model would look reproducible while being
/// reproducible only by whoever still remembered the flags.
///
/// `None` for a model saved before the config was recorded, for the same
/// reason the seed is optional — every field of [`FitConfig`] has a legal
/// value, so there is no configuration that could stand for "unknown".
#[serde(default)]
pub config: Option<crate::fit::FitConfig>,
pub columns: Vec<ColumnSpec>,
pub feature_names: Vec<String>,
pub target: TargetModel,
@@ -136,6 +154,13 @@ pub struct Dataset {
pub rows: Vec<Vec<f32>>,
pub target: Target,
pub columns: Vec<ColumnSpec>,
/// Human-readable notices about decisions [`preprocess`] made that the
/// caller should see — currently, raw columns dropped for having no data
/// at all. They are returned rather than printed: this is a library, and
/// only the caller knows whether its process has a usable stderr, an
/// alternate-screen TUI, or a status line to put them in. Same shape as
/// `improvise-io`'s `build_model_with_warnings`.
pub warnings: Vec<String>,
}
/// A cell counts as missing if it's blank or a common null token.
@@ -413,6 +438,7 @@ pub fn preprocess(header: &[String], rows: &[Vec<String>], ov: &Overrides) -> Da
let mut ranges = Vec::new();
let mut cols: Vec<Vec<f32>> = Vec::new();
let mut columns = Vec::new();
let mut warnings = Vec::new();
for (j, col_name) in header.iter().enumerate().take(n_feat) {
if ov.ignore.contains(col_name) {
continue;
@@ -420,7 +446,7 @@ pub fn preprocess(header: &[String], rows: &[Vec<String>], ov: &Overrides) -> Da
let cells = column(rows, j);
let present: Vec<&str> = cells.iter().copied().filter(|c| !is_missing(c)).collect();
if present.is_empty() {
eprintln!("(column {col_name:?} is entirely missing — skipped)");
warnings.push(format!("column {col_name:?} is entirely missing — skipped"));
continue;
}
// Forced type, else infer from the present cells.
@@ -467,6 +493,7 @@ pub fn preprocess(header: &[String], rows: &[Vec<String>], ov: &Overrides) -> Da
rows: x,
target,
columns,
warnings,
}
}
@@ -528,12 +555,28 @@ pub fn encode_row(columns: &[ColumnSpec], header: &[String], row: &[String]) ->
#[cfg(test)]
mod tests {
/// A config in which *every* field differs from `FitConfig::default()`, so
/// a round trip that silently substituted the default would be visible.
fn a_fit_config() -> crate::fit::FitConfig {
crate::fit::FitConfig {
pop: 37,
generations: 11,
elite: 3,
immigrants: 2,
max_nodes: 19,
penalty: 0.25,
restarts: 4,
}
}
/// A `Model` populated so every field family carries a non-default value:
/// both `ColumnKind` shapes that impute, a one-hot list, and a real tree.
/// both `ColumnKind` shapes that impute, a one-hot list, a real tree, and
/// a fully non-default `FitConfig`.
fn a_saved_model(seed: Option<u64>) -> Model {
Model {
target_name: "Score".to_string(),
seed,
config: Some(a_fit_config()),
columns: vec![
ColumnSpec {
name: "amount".to_string(),
@@ -604,6 +647,44 @@ mod tests {
assert_ne!(zero.seed, a_saved_model(None).seed);
}
/// The seed on its own is *necessary but 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 in fact
/// being reproducible only by whoever still remembers the flags. The config
/// therefore travels with it, and every field has to survive the trip —
/// a round trip that quietly substituted `FitConfig::default()` would pass
/// a test built on a default-constructed value.
#[test]
fn a_saved_model_round_trips_with_its_whole_ga_config() {
let json = serde_json::to_string(&a_saved_model(Some(0xABCD_1234))).unwrap();
let back: Model = serde_json::from_str(&json).unwrap();
assert_eq!(back.config, Some(a_fit_config()));
assert_ne!(
back.config,
Some(crate::fit::FitConfig::default()),
"the populated config must not be the default one"
);
}
/// A model saved before the config was recorded must still load, for the
/// same reason a pre-seed one must: `#[serde(default)]` is easy to drop.
#[test]
fn a_model_saved_before_configs_still_loads() {
let json = serde_json::to_string(&a_saved_model(Some(7))).unwrap();
let mut value: serde_json::Value = serde_json::from_str(&json).unwrap();
value
.as_object_mut()
.expect("a model is a JSON object")
.remove("config");
let back: Model = serde_json::from_value(value).unwrap();
assert_eq!(
back.config, None,
"an absent config is unknown, not the default one — every \
FitConfig value is legal, so none can stand for \"unknown\""
);
assert_eq!(back.seed, Some(7), "and the seed is untouched");
}
use super::*;
fn table(header: &[&str], rows: &[&[&str]]) -> (Vec<String>, Vec<Vec<String>>) {
@@ -707,6 +788,44 @@ mod tests {
assert_eq!(d.feature_names, ["size"]);
}
/// A column with nothing in it has no type to infer, so it is dropped —
/// and the caller has to be *told*, because a silently narrower feature
/// layout is exactly the kind of thing a user wants to know about.
///
/// `preprocess` used to say so with an `eprintln!`. That was correct when
/// this code was `examples/fit_csv.rs`, but a library does not own the
/// caller's stderr: a TUI writing to 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. So the notice comes back as data,
/// the way `improvise-io`'s `build_model_with_warnings` already does.
#[test]
fn an_entirely_missing_column_is_skipped_and_reported_as_data() {
let (h, rows) = table(
&["ghost", "size", "price"],
&[&["", "1", "10"], &["NA", "2", "20"], &["", "3", "30"]],
);
let d = preprocess(&h, &rows, &Overrides::default());
assert_eq!(d.feature_names, ["size"], "the empty column is dropped");
assert_eq!(d.warnings.len(), 1, "and the caller is told which");
assert!(
d.warnings[0].contains("ghost") && d.warnings[0].contains("entirely missing"),
"{:?}",
d.warnings[0]
);
}
/// The flip side: a clean table produces no warnings, so a caller can print
/// the vec unconditionally without inventing noise.
#[test]
fn a_clean_table_produces_no_warnings() {
let (h, rows) = table(&["size", "price"], &[&["1", "10"], &["2", "20"]]);
assert!(
preprocess(&h, &rows, &Overrides::default())
.warnings
.is_empty()
);
}
#[test]
fn a_non_numeric_target_becomes_class_codes() {
let (h, rows) = table(
+474
View File
@@ -0,0 +1,474 @@
//! The search: the loop that drives the operators in [`crate`] towards a tree
//! that explains the data, plus the two things that make its answer *credible*
//! rather than merely plausible — held-out error and permutation importance.
//!
//! [`crate::breed`] steps one generation and stays fitness-agnostic on purpose.
//! Everything above it — scoring, sorting, elitism, the parsimony penalty,
//! restarts, stopping — used to live in `examples/fit_csv.rs`, so every
//! consumer had to write it again. [`fit`] is that loop, and [`FitConfig`] is
//! the set of knobs it reads.
//!
//! Nothing here prints. Cross-validation hands back the pooled
//! `(prediction, actual)` pairs and importance hands back a score per feature;
//! turning those into a table is the caller's business, exactly as
//! [`crate::data::Dataset::warnings`] leaves the choice of stderr to the caller.
//!
//! Every stochastic choice comes from the caller's [`Rng`], in a fixed order,
//! so one seeded generator run through `fit` — or through a whole
//! cross-validate-then-fit-then-explain sequence — reproduces exactly.
use crate::{GaConfig, Population, Rng, Spec, Tree, breed, eval, random_tree, simplify, size};
/// What shapes a [`fit`]: the GA's own knobs plus the two that decide what
/// "best" means (`penalty`) and how hard to look (`restarts`).
///
/// The defaults are the ones the `fit_csv` CLI has always used. It is
/// serializable because [`crate::data::Model`] records it next to the seed —
/// a seed alone does not reproduce a fit.
#[derive(Clone, Copy, Debug, PartialEq, serde::Serialize, serde::Deserialize)]
pub struct FitConfig {
/// Target population size.
pub pop: usize,
/// Generations to step per restart.
pub generations: usize,
/// Top members carried over unchanged each generation.
pub elite: usize,
/// Fully-random immigrants injected each generation (diversity).
pub immigrants: usize,
/// Node cap, keeping the fitted tree readable.
pub max_nodes: usize,
/// Per-node cost added to the error, so a tree that fits no better but is
/// larger loses. This is what keeps a fit general rather than memorised.
pub penalty: f32,
/// Independent restarts to keep the best of. GP is stochastic, so one run
/// can land in a poor basin.
pub restarts: usize,
}
impl Default for FitConfig {
fn default() -> Self {
Self {
pop: 80,
generations: 600,
elite: 8,
immigrants: 8,
max_nodes: 25,
penalty: 0.01,
restarts: 1,
}
}
}
impl FitConfig {
/// The per-generation knobs [`breed`] takes.
pub fn ga(&self) -> GaConfig {
GaConfig {
pop: self.pop,
elite: self.elite,
immigrants: self.immigrants,
max_nodes: self.max_nodes,
}
}
/// The objective a [`fit`] minimises: mean squared error plus the parsimony
/// cost. Public because a caller comparing its own candidate against a fit
/// has to score it the same way.
pub fn cost(&self, t: &Tree, x: &[Vec<f32>], y: &[f32]) -> f32 {
mse(t, x, y) + self.penalty * size(t) as f32
}
}
/// Mean squared error of a tree over the rows.
///
/// The count comes from the rows actually scored, not from `x.len()`: `zip`
/// stops at the shorter of the two, so a denominator taken from one of them
/// could disagree with the numerator and quietly return a mean that is too
/// small. That is improvise-n3wt's hazard (gotcha 1) — a length supplied
/// beside the data rather than taken from it. Where the two slices do match,
/// which is everywhere in this crate, the result is unchanged.
pub fn mse(t: &Tree, x: &[Vec<f32>], y: &[f32]) -> f32 {
let (sse, scored) = x
.iter()
.zip(y)
.fold((0.0f32, 0usize), |(sse, n), (f, &yi)| {
let e = eval(t, f) - yi;
(sse + e * e, n + 1)
});
sse / scored as f32
}
/// Evolve a tree minimising [`FitConfig::cost`] over `x`/`y`, returning the
/// best **simplified** champion across `cfg.restarts` independent restarts.
///
/// The population is seeded from one random tree, so member 0 is an ancestor
/// the run can never score worse than; each generation is scored, sorted
/// best-first and handed to [`breed`].
pub fn fit(x: &[Vec<f32>], y: &[f32], spec: &Spec, cfg: &FitConfig, rng: &mut impl Rng) -> Tree {
let ga = cfg.ga();
let mut best: Option<(f32, Tree)> = None;
for _ in 0..cfg.restarts.max(1) {
let mut pop = Population::seed(&random_tree(spec, 4, rng), cfg.pop, spec, rng);
let mut run_best = pop.members[0].clone();
let mut run_cost = cfg.cost(&run_best, x, y);
for _ in 0..cfg.generations {
let mut scored: Vec<(f32, Tree)> = pop
.members
.iter()
.map(|t| (cfg.cost(t, x, y), t.clone()))
.collect();
scored.sort_by(|a, b| a.0.partial_cmp(&b.0).unwrap_or(std::cmp::Ordering::Equal));
if scored[0].0 < run_cost {
run_cost = scored[0].0;
run_best = scored[0].1.clone();
}
let best_first: Vec<Tree> = scored.into_iter().map(|(_, t)| t).collect();
pop.members = breed(best_first, &ga, spec, rng);
}
let simplified = simplify(&run_best, spec);
let final_cost = cfg.cost(&simplified, x, y);
if best.as_ref().is_none_or(|(c, _)| final_cost < *c) {
best = Some((final_cost, simplified));
}
}
best.expect("at least one restart always runs").1
}
/// The class whose tree scores highest for this row (one-vs-rest prediction).
pub fn argmax(trees: &[Tree], row: &[f32]) -> usize {
(0..trees.len())
.max_by(|&a, &b| {
eval(&trees[a], row)
.partial_cmp(&eval(&trees[b], row))
.unwrap_or(std::cmp::Ordering::Equal)
})
.unwrap()
}
/// Fraction of rows the one-vs-rest forest misclassifies.
pub fn misclass_rate(trees: &[Tree], x: &[Vec<f32>], codes: &[usize]) -> f32 {
let wrong = (0..x.len())
.filter(|&i| argmax(trees, &x[i]) != codes[i])
.count();
wrong as f32 / x.len() as f32
}
/// Standardize a target to ≈ zero-mean / unit-std, returning (normalized, mean,
/// std). The crate's leaf scores live in a small range, so a large-magnitude
/// target has to be scaled in or it cannot be reached by leaf mutations; splits
/// are scale-free, and only the leaves are affine-mapped back to real units.
pub fn standardize(y: &[f32]) -> (Vec<f32>, f32, f32) {
let n = y.len() as f32;
let mean = y.iter().sum::<f32>() / n;
let std = (y.iter().map(|v| (v - mean).powi(2)).sum::<f32>() / n)
.sqrt()
.max(1e-6);
(y.iter().map(|v| (v - mean) / std).collect(), mean, std)
}
/// Split `0..n` into `k` folds after a deterministic shuffle, so held-out rows
/// are a representative sample rather than a contiguous block.
///
/// `k` arrives from a host — a CLI flag, in this crate's only caller — so zero
/// is a number to answer, not an impossible state. One fold is the answer the
/// shape already gives; the alternative was `out[pos % 0]`, a library function
/// taking its host down with it. [`fit`] spells the same `max(1)` over
/// `restarts`, for the same reason.
pub fn folds(n: usize, k: usize, rng: &mut impl Rng) -> Vec<Vec<usize>> {
let k = k.max(1);
let mut idx: Vec<usize> = (0..n).collect();
for i in (1..n).rev() {
idx.swap(i, rng.below(i + 1));
}
let mut out = vec![Vec::new(); k];
for (pos, &row) in idx.iter().enumerate() {
out[pos % k].push(row);
}
out
}
/// Sub-slice `x`/`y` to the given row indices.
fn take(x: &[Vec<f32>], y: &[f32], idx: &[usize]) -> (Vec<Vec<f32>>, Vec<f32>) {
(
idx.iter().map(|&i| x[i].clone()).collect(),
idx.iter().map(|&i| y[i]).collect(),
)
}
/// `k`-fold CV for regression: for each fold, standardize + [`fit`] on the
/// other folds, predict the held-out rows (de-normalized to original units),
/// and pool `(prediction, actual)` across all folds.
///
/// Nothing here leaks the test fold — the mean/std come from the training rows
/// only. The pooled pairs are returned rather than reduced to a metric, so the
/// caller picks R², RMSE or whatever else it wants to show.
pub fn cv_regress(
x: &[Vec<f32>],
y: &[f32],
spec: &Spec,
cfg: &FitConfig,
k: usize,
rng: &mut impl Rng,
) -> Vec<(f32, f32)> {
let mut pooled = Vec::new();
for test in folds(y.len(), k, rng) {
let train: Vec<usize> = (0..y.len()).filter(|i| !test.contains(i)).collect();
let (tx, ty) = take(x, y, &train);
let (ty_norm, mean, std) = standardize(&ty);
let tree = fit(&tx, &ty_norm, spec, cfg, rng);
for i in test {
pooled.push((eval(&tree, &x[i]) * std + mean, y[i]));
}
}
pooled
}
/// `k`-fold CV for classification: pool `(predicted_code, actual_code)` over the
/// held-out rows, fitting one-vs-rest trees on each training split.
pub fn cv_classify(
x: &[Vec<f32>],
codes: &[usize],
n_classes: usize,
spec: &Spec,
cfg: &FitConfig,
k: usize,
rng: &mut impl Rng,
) -> Vec<(usize, usize)> {
let mut pooled = Vec::new();
for test in folds(codes.len(), k, rng) {
let train: Vec<usize> = (0..codes.len()).filter(|i| !test.contains(i)).collect();
let trees: Vec<Tree> = (0..n_classes)
.map(|c| {
let y: Vec<f32> = train
.iter()
.map(|&i| (codes[i] == c) as i32 as f32)
.collect();
let tx: Vec<Vec<f32>> = train.iter().map(|&i| x[i].clone()).collect();
fit(&tx, &y, spec, cfg, rng)
})
.collect();
for i in test {
pooled.push((argmax(&trees, &x[i]), codes[i]));
}
}
pooled
}
/// Model-agnostic **permutation importance**: how much the model's error grows
/// when each feature column is shuffled across rows (severing its link to the
/// target), averaged over `repeats`. It calls only the black-box `err` closure,
/// so the same routine serves a regression tree and a classification forest —
/// no access to tree internals needed.
pub fn permutation_importance(
err: impl Fn(&[Vec<f32>]) -> f32,
x: &[Vec<f32>],
n_feat: usize,
repeats: usize,
rng: &mut impl Rng,
) -> Vec<f32> {
let base = err(x);
let n = x.len();
let mut work = x.to_vec();
let mut imp = vec![0.0; n_feat];
for (j, slot) in imp.iter_mut().enumerate() {
let col: Vec<f32> = x.iter().map(|r| r[j]).collect();
let mut acc = 0.0;
for _ in 0..repeats.max(1) {
let mut idx: Vec<usize> = (0..n).collect();
for i in (1..n).rev() {
idx.swap(i, rng.below(i + 1));
}
for i in 0..n {
work[i][j] = col[idx[i]];
}
acc += err(&work) - base;
}
for (i, &c) in col.iter().enumerate() {
work[i][j] = c; // restore the column for the next feature
}
*slot = (acc / repeats.max(1) as f32).max(0.0);
}
imp
}
#[cfg(test)]
mod tests {
use super::*;
use crate::{SplitMix64, render};
const RANGES: [(f32, f32); 2] = [(0.0, 1.0), (0.0, 10.0)];
const NAMES: [&str; 2] = ["flag", "amount"];
fn spec() -> Spec<'static> {
Spec {
ranges: &RANGES,
names: &NAMES,
}
}
/// A rule a tree can express exactly: the target steps at amount > 5.
fn rows() -> (Vec<Vec<f32>>, Vec<f32>) {
(0..20)
.map(|i| {
let amount = i as f32 / 2.0;
(
vec![(i % 2) as f32, amount],
if amount > 5.0 { 2.0 } else { -1.0 },
)
})
.unzip()
}
/// Small enough to keep the suite fast, large enough that every random
/// choice — seeding, mutation, crossover, immigrants — is still made.
fn cfg() -> FitConfig {
FitConfig {
pop: 16,
generations: 20,
elite: 4,
immigrants: 2,
max_nodes: 15,
penalty: 0.01,
restarts: 1,
}
}
/// The whole reason `Rng` is a caller-supplied trait: a fit writes a rule
/// into a user's document, so the same seed over the same data and the same
/// config has to give back the same tree.
#[test]
fn the_same_seed_and_config_fit_the_same_tree() {
let (x, y) = rows();
let one = fit(&x, &y, &spec(), &cfg(), &mut SplitMix64::new(0xABCD_1234));
let two = fit(&x, &y, &spec(), &cfg(), &mut SplitMix64::new(0xABCD_1234));
assert_eq!(render(&one, &spec()), render(&two, &spec()));
}
/// And the seed is what varies it — otherwise "reproducible" could just
/// mean the search never moved.
#[test]
fn a_different_seed_fits_a_different_tree() {
let (x, y) = rows();
let one = fit(&x, &y, &spec(), &cfg(), &mut SplitMix64::new(0xABCD_1234));
let two = fit(&x, &y, &spec(), &cfg(), &mut SplitMix64::new(0x9999_0001));
assert_ne!(render(&one, &spec()), render(&two, &spec()));
}
/// The search actually searches: the champion beats the tree the run
/// started from. Without this, a `fit` that returned its seed tree
/// unchanged would still satisfy both tests above.
#[test]
fn a_fit_improves_on_the_tree_it_started_from() {
let (x, y) = rows();
let c = cfg();
for seed in [0xABCD_1234u64, 0x9999_0001, 7] {
// The seed tree is the first thing `fit` draws from the generator,
// so drawing it the same way here reproduces the run's starting
// point exactly.
let start = random_tree(&spec(), 4, &mut SplitMix64::new(seed));
let fitted = fit(&x, &y, &spec(), &c, &mut SplitMix64::new(seed));
let (before, after) = (c.cost(&start, &x, &y), c.cost(&fitted, &x, &y));
assert!(
after < before,
"seed {seed:#x}: the fit cost {after}, its starting tree cost {before}"
);
}
}
/// The config is not decoration: a fit under different knobs is a different
/// fit, which is why `data::Model` has to record it next to the seed.
#[test]
fn the_config_changes_the_fit_at_a_fixed_seed() {
let (x, y) = rows();
let lean = FitConfig {
penalty: 2.0,
..cfg()
};
let loose = fit(&x, &y, &spec(), &cfg(), &mut SplitMix64::new(4));
let small = fit(&x, &y, &spec(), &lean, &mut SplitMix64::new(4));
assert!(
size(&small) <= size(&loose),
"a heavy parsimony penalty should not grow the tree ({} vs {})",
size(&small),
size(&loose)
);
assert_ne!(render(&small, &spec()), render(&loose, &spec()));
}
#[test]
fn folds_partition_every_row_exactly_once() {
let mut rng = SplitMix64::new(11);
let f = folds(20, 5, &mut rng);
assert_eq!(f.len(), 5);
let mut all: Vec<usize> = f.concat();
all.sort();
assert_eq!(all, (0..20).collect::<Vec<_>>());
}
/// `k` reaches here from a CLI flag, so zero is a value a caller can
/// supply rather than an impossible state. `out[pos % k]` divided by it,
/// which panicked — a public library function taking a host's number down
/// with it. One fold is the answer the shape already gives: `fit` spells
/// the same `max(1)` over `restarts`.
#[test]
fn asking_for_no_folds_yields_one_rather_than_dividing_by_zero() {
let mut rng = SplitMix64::new(11);
let f = folds(6, 0, &mut rng);
assert_eq!(f.len(), 1);
let mut all = f.concat();
all.sort();
assert_eq!(all, (0..6).collect::<Vec<_>>());
}
/// `mse` summed over `zip(x, y)` — which stops at the shorter — and then
/// divided by `x.len()`. The two disagreeing is the improvise-n3wt hazard
/// (gotcha 1) in miniature: a denominator supplied alongside the data
/// rather than taken from it, so a caller could get them out of step and
/// be handed a number that is quietly too small instead of an error.
/// Taking the count from the rows actually scored removes the second
/// length; the truncation itself needs the type change filed as
/// improvise-mse-shape, and is not fixed here.
#[test]
fn mse_averages_over_the_rows_it_actually_scored() {
let (x, y) = rows();
let mut rng = SplitMix64::new(3);
let t = random_tree(&spec(), 3, &mut rng);
let short = y.len() - 1;
assert_eq!(
mse(&t, &x, &y[..short]),
mse(&t, &x[..short], &y[..short]),
"the mean was taken over rows the zip never reached"
);
}
/// Cross-validated predictions come from trees that never saw the row they
/// score — that is the whole point, and it is easy to lose in a refactor.
#[test]
fn cv_regress_pools_one_prediction_per_row() {
let (x, y) = rows();
let mut rng = SplitMix64::new(5);
let pooled = cv_regress(&x, &y, &spec(), &cfg(), 4, &mut rng);
assert_eq!(pooled.len(), y.len());
let mut actuals: Vec<f32> = pooled.iter().map(|&(_, a)| a).collect();
let mut expected = y.clone();
actuals.sort_by(|a, b| a.partial_cmp(b).unwrap());
expected.sort_by(|a, b| a.partial_cmp(b).unwrap());
assert_eq!(actuals, expected);
}
/// An irrelevant feature earns no importance, a load-bearing one does.
#[test]
fn permutation_importance_finds_the_feature_the_tree_uses() {
let (x, y) = rows();
// A tree that reads feature 1 only.
let tree = Tree::Split {
feat: 1,
thresh: 5.25,
lo: Box::new(Tree::Leaf(-1.0)),
hi: Box::new(Tree::Leaf(2.0)),
};
let mut rng = SplitMix64::new(13);
let imp = permutation_importance(|xm| mse(&tree, xm, &y), &x, 2, 3, &mut rng);
assert_eq!(imp[0], 0.0, "the unused feature carries no importance");
assert!(imp[1] > 0.0, "the split feature does: {:?}", imp);
}
}
+14 -1
View File
@@ -23,7 +23,19 @@
//! row the same way. It is data-shaped, not domain-shaped, so it stays as
//! problem-agnostic as the rest.
//!
//! ## Typical loop
//! ## Driving the search
//!
//! When the fitness is "predict this target from these rows" — the [`data`]
//! case — [`fit`] already is the loop, along with the cross-validation and
//! permutation importance that say whether to believe its answer:
//!
//! ```ignore
//! let champion = fit::fit(&ds.rows, &y, &spec, &FitConfig::default(), &mut rng);
//! ```
//!
//! For any other fitness — playing a game, ranking configs — drive [`breed`]
//! yourself; it stays fitness-agnostic on purpose:
//!
//! ```ignore
//! let mut pop = Population::seed(&champion, cfg.pop, &spec, &mut rng);
//! for _ in 0..generations {
@@ -41,6 +53,7 @@
use serde::{Deserialize, Serialize};
pub mod data;
pub mod fit;
/// The randomness the GP operators need. Implement it over your own *seeded*
/// generator so evolution is reproducible — the crate never pulls in an RNG
+47 -9
View File
@@ -22,6 +22,7 @@ use crate::format::parse_number_format;
use crate::import::wizard::ImportWizard;
use crate::model::cell::CellKey;
use crate::persistence;
use crate::ui::project::Dirty;
use crate::ui::viewport::{compute_col_widths, compute_row_header_width, compute_visible_cols};
use crate::view::GridLayout;
use crate::workbook::Workbook;
@@ -117,6 +118,13 @@ pub struct App {
/// "advance at bottom-right" short-circuits the trailing
/// `EnterEditAtCursor` in a `CommitAndAdvance` chain.
pub abort_effects: bool,
/// **Session-local**: how many times the whole document has been
/// replaced. Bumped by `install_workbook` and by nothing else, so a
/// client mirror can tell that the document it was attached to is gone
/// without the effect that replaced it having to reach the session list
/// (improvise-cqq). It is not part of `ViewState`: it describes the
/// document's identity over this process's lifetime, not a client's view.
document_generation: u64,
/// **Config**: keymap configuration assembled at startup by
/// `KeymapSet::default_keymaps()`. Read every keypress but never
/// mutated during a session.
@@ -171,6 +179,7 @@ impl App {
term_height: 24,
layout,
abort_effects: false,
document_generation: 0,
keymap_set: KeymapSet::default_keymaps(),
storage,
}
@@ -204,6 +213,17 @@ impl App {
self.view_state.active_view = workbook.active_view.clone();
self.view_state.cursors.clear();
self.model_state.workbook = workbook;
// Every connected client's mirror is scoped to the outgoing document
// in the same way, and the effect that got here cannot reach the
// session list. Bumping the generation is how the reset reaches them:
// `ClientSession::project` compares it and reseeds.
self.document_generation += 1;
}
/// How many times this app's document has been replaced. A session that
/// was seeded at an earlier generation is holding another document.
pub fn document_generation(&self) -> u64 {
self.document_generation
}
/// Point the view slice at `name` and switch the workbook to it. The
@@ -326,10 +346,17 @@ impl App {
}
}
pub fn apply_effects(&mut self, effects: Vec<Box<dyn super::effect::Effect>>) {
/// Apply a batch of effects and report which widgets it could have
/// changed, so a host with connected clients knows what to project
/// (improvise-cqq). The `Dirty` accumulates *inside* the loop: an
/// aborted batch must not claim the effects it never ran.
pub fn apply_effects(&mut self, effects: Vec<Box<dyn super::effect::Effect>>) -> Dirty {
self.abort_effects = false;
let mut dirty = Dirty::NOTHING;
for effect in effects {
let kind = effect.kind();
effect.apply(self);
dirty = dirty.union(Dirty::of(kind));
if self.abort_effects {
// AbortChain (or another abort-setting effect) requested
// that the rest of this batch be skipped. Reset the flag so
@@ -339,6 +366,7 @@ impl App {
}
}
self.rebuild_layout();
dirty
}
/// True when the model has no user-defined categories (show welcome/help).
@@ -377,6 +405,18 @@ impl App {
}
pub fn handle_key(&mut self, key: KeyEvent) -> Result<()> {
self.dispatch_key(key);
Ok(())
}
/// Resolve one keypress through the keymap and apply what it produces,
/// reporting which widgets changed.
///
/// The whole key path, with the projection half factored out: a host
/// with no connected clients (the native TUI) drops the `Dirty`, and one
/// with clients hands it to `Sessions::project_all`. `ui::reduce` is the
/// named door onto this (improvise-gxi).
pub fn dispatch_key(&mut self, key: KeyEvent) -> Dirty {
self.rebuild_layout();
// An armed prefix key takes priority. Resolve it back to its
@@ -389,11 +429,10 @@ impl App {
self.keymap_set
.dispatch_transient(&transient, &ctx, key.code, key.modifiers)
});
if let Some(effects) = effects {
self.apply_effects(effects);
}
// Whether matched or not, the prefix is consumed.
return Ok(());
return effects
.map(|effects| self.apply_effects(effects))
.unwrap_or(Dirty::NOTHING);
}
// Try mode keymap — if a binding matches, apply effects and return
@@ -401,10 +440,9 @@ impl App {
let ctx = self.cmd_context(key.code, key.modifiers);
self.keymap_set.dispatch(&ctx, key.code, key.modifiers)
};
if let Some(effects) = effects {
self.apply_effects(effects);
}
Ok(())
effects
.map(|effects| self.apply_effects(effects))
.unwrap_or(Dirty::NOTHING)
}
pub fn autosave_if_needed(&mut self) {
+2
View File
@@ -1,4 +1,6 @@
pub mod app;
pub mod cat_tree;
pub mod effect;
pub mod project;
pub mod reduce;
pub mod viewport;
File diff suppressed because it is too large Load Diff
+231
View File
@@ -0,0 +1,231 @@
//! The one place a command is applied to the authoritative state
//! (improvise-gxi).
//!
//! Every deployment mode — the native TUI, a websocket server, a browser
//! worker — reaches the model through the functions here, so there is one
//! answer to "what does this input do" rather than one per host.
//!
//! Two doors, because input arrives in two shapes and only one of them is
//! data on the wire:
//!
//! - [`reduce_full`] takes a [`Command`], the wire vocabulary. Its
//! client→server half is exactly the set of view-slice mutations
//! (`decision(improvise-cqi, command_set_derived_not_invented)`), so a
//! command changes a client's view and, through the active view, what the
//! grid shows — it cannot yet express "set this cell".
//! - [`reduce_key`] takes a keypress and runs it through the keymap,
//! registry and effect pipeline. This is still the only door the model
//! mutations have, because no `Command` names them. Deriving that half of
//! the vocabulary from the `EffectKind::Model` effects is its own piece of
//! work; inventing the variants ahead of it is what the decision above
//! forbids.
//!
//! Both return the commands to send onward: the projections for the widgets
//! whose content is the document's rather than one client's. The grid is
//! per-viewport and comes from [`Sessions::project_all`](super::project::Sessions::project_all)
//! instead — a host with connected clients uses the [`Dirty`] the inner
//! functions return and projects per session; a host with none, like the
//! native TUI, drops what it gets.
use improvise_protocol::{Command, reduce_view};
use crate::command::key::KeyEvent;
use crate::ui::app::App;
use crate::ui::project::{Dirty, document_projections};
/// Apply one wire command to the authoritative state, and say what it
/// dirtied.
///
/// `Command::SwitchView` does not go through `reduce_view` alone: the
/// document's "which view is active" and the client's must move together,
/// and `App::switch_to_view` is one of the only two writers allowed to touch
/// either (gotcha 29). Everything else is pure view slice.
///
/// An inbound `Command::Project` is ignored. Projections travel
/// server→client; a client that sends one is describing state it does not
/// own.
pub fn apply_command(app: &mut App, cmd: &Command) -> Dirty {
match cmd {
Command::Project(_) => Dirty::NOTHING,
Command::SwitchView(name) => {
app.switch_to_view(name);
app.rebuild_layout();
// The view list itself is unchanged — its names and axes belong
// to the document. What moved is which view the axes are read
// through, so the grid, the category panel's axis column and the
// tile bar all change.
Dirty {
grid: true,
category_tree: true,
tile_bar: true,
..Dirty::NOTHING
}
}
other => {
reduce_view(&mut app.view_state, other);
Dirty::NOTHING
}
}
}
/// Apply one wire command and return what every client must be told.
///
/// The signature the three hosts share. The returned commands are the
/// document-scoped projections; per-client grid updates need a session's
/// viewport and come from `Sessions::project_all`, driven by the [`Dirty`]
/// that [`apply_command`] returns.
pub fn reduce_full(app: &mut App, cmd: &Command) -> Vec<Command> {
let dirty = apply_command(app, cmd);
document_projections(app, dirty)
.into_iter()
.map(Command::Project)
.collect()
}
/// The same, for a keypress: resolve it through the keymap, run the
/// commands, apply the effects, and report what to send onward.
pub fn reduce_key(app: &mut App, key: KeyEvent) -> Vec<Command> {
let dirty = app.dispatch_key(key);
document_projections(app, dirty)
.into_iter()
.map(Command::Project)
.collect()
}
#[cfg(test)]
mod tests {
use super::*;
use crate::model::cell::CellValue;
use crate::ui::app::AppMode;
use crate::view::View;
use crate::workbook::Workbook;
use improvise_protocol::Projection;
fn app() -> App {
let mut wb = Workbook::new("Test");
wb.add_category("Region").unwrap();
wb.add_item("Region", "North").unwrap();
wb.views.insert("Second".to_string(), View::new("Second"));
App::new(wb, None)
}
/// The command stream is the only way a client's view slice moves, and
/// it moves the same way on both ends.
#[test]
fn view_slice_commands_land_in_the_authoritative_view_state() {
let mut app = app();
let outbound = reduce_full(&mut app, &Command::SetStatus("saved".into()));
assert_eq!(app.view_state.status_msg, "saved");
assert!(outbound.is_empty(), "a status line reaches no widget");
reduce_full(&mut app, &Command::SetSelected { row: 2, col: 3 });
assert_eq!(app.cursor().selected, (2, 3));
reduce_full(&mut app, &Command::ChangeMode(AppMode::Help));
assert!(matches!(app.view_state.mode, AppMode::Help));
reduce_full(&mut app, &Command::SetYanked(Some(CellValue::Number(1.0))));
assert_eq!(app.view_state.yanked, Some(CellValue::Number(1.0)));
}
/// The two copies of "which view is active" must move together
/// (gotcha 29). Folding `SwitchView` through `reduce_view` alone would
/// move the client's and leave the document's behind, and the cursor map
/// would then be read under the wrong key.
#[test]
fn switch_view_moves_the_document_and_the_client_together() {
let mut app = app();
reduce_full(&mut app, &Command::SwitchView("Second".into()));
assert_eq!(app.view_state.active_view, "Second");
assert_eq!(app.model_state.workbook.active_view, "Second");
}
/// Switching views changes what the axis-bearing widgets read, so they
/// are told; the list of views itself did not change, so it is not.
#[test]
fn switching_views_projects_the_axis_widgets_only() {
let mut app = app();
let outbound = reduce_full(&mut app, &Command::SwitchView("Second".into()));
assert!(
outbound
.iter()
.any(|c| matches!(c, Command::Project(Projection::CategoryTree(_)))),
"the category panel's axis column moved: {outbound:?}"
);
assert!(
outbound
.iter()
.any(|c| matches!(c, Command::Project(Projection::TileBar(_)))),
"the tile bar's axes moved: {outbound:?}"
);
assert!(
!outbound
.iter()
.any(|c| matches!(c, Command::Project(Projection::ViewList(_)))),
"the set of views did not change: {outbound:?}"
);
}
/// Projections travel one way. A client that sends one back must not be
/// able to write it into the server's state.
#[test]
fn an_inbound_projection_is_ignored() {
use improvise_protocol::GridUpdate;
let mut app = app();
let before = format!("{:?}", app.view_state);
let outbound = reduce_full(
&mut app,
&Command::Project(Projection::Grid(GridUpdate::RowLabels(vec!["x".into()]))),
);
assert!(outbound.is_empty());
assert_eq!(before, format!("{:?}", app.view_state));
}
/// The key path is the model's door, and it reports the widgets it
/// touched so a host can project them.
#[test]
fn a_keypress_that_changes_the_model_projects_it() {
use crate::command::key::{KeyCode, KeyModifiers};
let mut app = app();
app.view_state.mode = AppMode::category_add();
app.view_state
.buffers
.insert("category".to_string(), "Product".to_string());
let outbound = reduce_key(
&mut app,
KeyEvent {
code: KeyCode::Enter,
modifiers: KeyModifiers::NONE,
},
);
assert!(
app.model_state.workbook.model.category("Product").is_some(),
"the key path still reaches the model"
);
assert!(
outbound
.iter()
.any(|c| matches!(c, Command::Project(Projection::CategoryTree(_)))),
"adding a category must reach the category panel: {outbound:?}"
);
}
/// A keypress that moves only this client's cursor reaches no other
/// client (gotcha 3).
#[test]
fn a_view_only_keypress_projects_nothing() {
use crate::command::key::{KeyCode, KeyModifiers};
let mut app = app();
let outbound = reduce_key(
&mut app,
KeyEvent {
code: KeyCode::Char('j'),
modifiers: KeyModifiers::NONE,
},
);
assert!(
outbound.is_empty(),
"a cursor move is nobody else's: {outbound:?}"
);
}
}
+67 -24
View File
@@ -128,13 +128,15 @@ fn data_index_to_visual(items: &[AxisEntry], data_idx: usize) -> Option<usize> {
.nth(data_idx)
}
/// Drop the data items whose `keep` flag is false, keeping a group header
/// only when a data item that follows it survives.
/// Drop the data items `keep` rejects, retaining a group header only when a
/// data item that follows it survives.
///
/// `keep` is indexed by *data* position, so it must have one entry per
/// `DataItem` in `items` — the assertion is the single place that invariant
/// is checked for both axes.
fn prune_axis_items(items: &mut Vec<AxisEntry>, keep: &[bool]) {
/// `keep` is asked about each `DataItem` by its *data* index — the position it
/// has in `data_items(items)`, which is what every public `row`/`col` argument
/// means. The count of those indices comes from `items` itself, so a caller
/// has no parallel length to get wrong and debug and release builds cannot
/// disagree about an out-of-range flag.
fn prune_axis_items(items: &mut Vec<AxisEntry>, keep: impl Fn(usize) -> bool) {
let mut kept = Vec::new();
let mut pending_header: Option<AxisEntry> = None;
let mut data_idx = 0usize;
@@ -144,7 +146,7 @@ fn prune_axis_items(items: &mut Vec<AxisEntry>, keep: &[bool]) {
pending_header = Some(entry);
}
AxisEntry::DataItem(_) => {
if keep.get(data_idx).copied().unwrap_or(false) {
if keep(data_idx) {
if let Some(h) = pending_header.take() {
kept.push(h);
}
@@ -154,11 +156,6 @@ fn prune_axis_items(items: &mut Vec<AxisEntry>, keep: &[bool]) {
}
}
}
debug_assert_eq!(
data_idx,
keep.len(),
"keep flags must be parallel to the axis's data items"
);
*items = kept;
}
@@ -403,17 +400,16 @@ impl GridLayout {
}
}
// Which data-row indices are non-empty?
let keep_row: Vec<bool> = (0..rc)
.map(|ri| (0..cc).any(|ci| has_value[ri][ci]))
.collect();
// Which data-col indices are non-empty?
let keep_col: Vec<bool> = (0..cc)
.map(|ci| (0..rc).any(|ri| has_value[ri][ci]))
.collect();
prune_axis_items(&mut self.row_items, &keep_row);
prune_axis_items(&mut self.col_items, &keep_col);
// A data row survives if any of its columns has content, a data
// column if any of its rows does. Pruning the rows does not touch
// `col_items` or `has_value`, so the column pass still reads the
// original grid.
prune_axis_items(&mut self.row_items, |ri| {
(0..cc).any(|ci| has_value[ri][ci])
});
prune_axis_items(&mut self.col_items, |ci| {
(0..rc).any(|ri| has_value[ri][ci])
});
}
/// Whether this layout is in records mode.
@@ -609,7 +605,10 @@ fn cross_product(model: &Model, view: &View, cats: &[String]) -> Vec<AxisEntry>
#[cfg(test)]
mod tests {
use super::{AxisEntry, GridLayout, NEW_MEASURE_COL, RecordCol, synthetic_record_info};
use super::{
AxisEntry, GridLayout, NEW_MEASURE_COL, RecordCol, data_items, prune_axis_items,
synthetic_record_info,
};
use crate::model::cell::{CellKey, CellValue};
use crate::view::Axis;
use crate::workbook::Workbook;
@@ -640,6 +639,50 @@ mod tests {
wb
}
/// `prune_axis_items` asks `keep` about *data* indices — the positions a
/// `row`/`col` argument means — so a group header between two data items
/// must not shift the numbering the predicate sees. The predicate's domain
/// comes from `items` itself, so no caller can hand over a flag sequence
/// whose length disagrees with the axis.
#[test]
fn prune_axis_items_indexes_data_positions_not_visual_ones() {
let mut items = vec![
AxisEntry::DataItem(vec!["A".into()]),
AxisEntry::GroupHeader {
cat_name: "C".into(),
group_name: "G".into(),
},
AxisEntry::DataItem(vec!["B".into()]),
AxisEntry::DataItem(vec!["C".into()]),
];
// Data index 1 is "B": the header does not consume an index. If the
// predicate saw visual indices, "A" would be the one dropped.
prune_axis_items(&mut items, |idx| idx != 1);
let survivors: Vec<&str> = data_items(&items).map(|v| v[0].as_str()).collect();
assert_eq!(survivors, vec!["A", "C"]);
// "C" survived, so its group header is retained.
assert!(items.iter().any(|e| matches!(
e,
AxisEntry::GroupHeader { group_name, .. } if group_name == "G"
)));
}
/// A group header is dropped when every data item under it is pruned.
#[test]
fn prune_axis_items_drops_a_header_whose_items_all_go() {
let mut items = vec![
AxisEntry::GroupHeader {
cat_name: "C".into(),
group_name: "G".into(),
},
AxisEntry::DataItem(vec!["A".into()]),
AxisEntry::DataItem(vec!["B".into()]),
];
prune_axis_items(&mut items, |_| false);
assert!(items.is_empty());
}
#[test]
fn prune_empty_removes_all_empty_columns_in_pivot_mode() {
let mut wb = Workbook::new("T");
+4 -26
View File
@@ -241,34 +241,12 @@ fn strip_string_quotes(s: &str) -> String {
s[1..s.len() - 1].to_string()
}
/// Strip surrounding pipes and apply backslash escapes: `\|` → `|`,
/// `\\` → `\`, `\n` → newline, and any other `\X` is preserved verbatim.
/// Matches the escape semantics documented in src/persistence/improv.pest.
/// Strip surrounding pipes and apply the escape table `quote::pipe_quote`
/// writes — the one copy of that rule, shared with `improvise-io`'s
/// `.improv` walker (improvise-i2m5).
fn unquote_pipe(s: &str) -> String {
debug_assert!(is_pipe_quoted(s));
let inner = &s[1..s.len() - 1];
let mut out = String::with_capacity(inner.len());
let mut chars = inner.chars();
while let Some(c) = chars.next() {
if c == '\\' {
// The grammar rule `"\\" ~ ANY` guarantees that a backslash
// inside a pipe-quoted identifier is always followed by another
// character, so `chars.next()` cannot be `None` here.
let escaped = chars.next().expect(GRAMMAR_INVARIANT);
match escaped {
'|' => out.push('|'),
'\\' => out.push('\\'),
'n' => out.push('\n'),
other => {
out.push('\\');
out.push(other);
}
}
} else {
out.push(c);
}
}
out
crate::quote::unescape_pipe_body(&s[1..s.len() - 1])
}
// ---- small helpers ------------------------------------------------------
+84 -7
View File
@@ -1,12 +1,19 @@
//! CL-style `|…|` quoting of names — the writing half of the convention
//! `formula.pest`'s `bare_ident` / `pipe_quoted` defines for reading.
//! CL-style `|…|` quoting of names — both halves of the convention
//! `formula.pest`'s `bare_ident` / `pipe_quoted` defines, and the one
//! `improv.pest`'s `bare_name` / `pipe_quoted` mirrors character for
//! character.
//!
//! This lives here because `improvise-formula` owns the grammar that says what
//! a bare name is, and because it is what every emitter needs: a name that came
//! from a user's data has to be spelled so the parser hands it back unchanged.
//! `improvise-gp` carried its own copy; so does
//! `improvise-io/src/persistence/quoting.rs`, and the two had already drifted
//! over the keyword rule below.
//! `improvise-gp` and `improvise-io` each carried their own copy, and all
//! three had drifted over the keyword rule below (improvise-i2m5).
//!
//! The two crates do not want the same entry point, so there are three:
//! [`quote_name`] for a name, [`pipe_quote`] for a value that must be quoted
//! whatever it looks like (a `.improv` text cell, so it cannot be mistaken for
//! a number), and [`unescape_pipe_body`] for reading either back. The escape
//! table is written once, in the last two.
/// Words the formula grammar gives a meaning of their own. A name spelled like
/// one is pipe-quoted even though it is otherwise a legal bare identifier.
@@ -37,9 +44,16 @@ pub fn quote_name(name: &str) -> String {
if is_bare_name(name) {
return name.to_string();
}
let mut out = String::with_capacity(name.len() + 2);
pipe_quote(name)
}
/// `s` pipe-quoted whatever it looks like. `.improv` text cell values go
/// through this rather than [`quote_name`], because a bare `42` there would
/// read back as a number.
pub fn pipe_quote(s: &str) -> String {
let mut out = String::with_capacity(s.len() + 2);
out.push('|');
for c in name.chars() {
for c in s.chars() {
match c {
'|' => out.push_str("\\|"),
'\\' => out.push_str("\\\\"),
@@ -51,6 +65,36 @@ pub fn quote_name(name: &str) -> String {
out
}
/// The inverse of [`pipe_quote`], over the body between the delimiters —
/// what both grammars' `pipe_inner` matches. `\|` → `|`, `\\` → `\`,
/// `\n` → newline; any other `\X` is preserved verbatim.
///
/// The trailing arms are unreachable from either grammar: `pipe_inner` is
/// `("\\" ~ ANY | !"|" ~ ANY)*`, so a backslash is always followed by a
/// character. They are written as a total function rather than a panic so
/// that a caller reaching this by some other route gets its bytes back.
pub fn unescape_pipe_body(body: &str) -> String {
let mut out = String::with_capacity(body.len());
let mut chars = body.chars();
while let Some(c) = chars.next() {
if c == '\\' {
match chars.next() {
Some('|') => out.push('|'),
Some('\\') => out.push('\\'),
Some('n') => out.push('\n'),
Some(other) => {
out.push('\\');
out.push(other);
}
None => out.push('\\'),
}
} else {
out.push(c);
}
}
out
}
#[cfg(test)]
mod tests {
use super::*;
@@ -85,6 +129,39 @@ mod tests {
assert_eq!(quote_name("two\nlines"), r"|two\nlines|");
}
/// The reading half inverts the writing half over every escape the
/// grammars define, plus the `\X` passthrough neither of them assigns a
/// meaning.
#[test]
fn unescaping_inverts_quoting() {
for s in [
"plain",
"a|b",
r"back\slash",
"two\nlines",
"",
"|",
r"\\",
"42",
] {
let quoted = pipe_quote(s);
let body = &quoted[1..quoted.len() - 1];
assert_eq!(unescape_pipe_body(body), s, "round trip for {s:?}");
}
assert_eq!(unescape_pipe_body(r"\q"), r"\q");
}
/// Before improvise-i2m5 the unescape table existed twice — here (as
/// `parser.rs::unquote_pipe`) and as
/// `improvise-io/src/persistence/quoting.rs::unescape_pipe`. The two
/// agreed on every input either grammar's `pipe_inner` can produce; they
/// differed only on a trailing lone backslash, which `pipe_inner` cannot
/// emit. This pins the surviving answer for that input.
#[test]
fn a_trailing_lone_backslash_is_kept() {
assert_eq!(unescape_pipe_body(r"a\"), r"a\");
}
/// The property the whole module exists for: a quoted name parses, and the
/// parser hands back the name that went in — as a target and as a
/// reference, which after the target-unquoting fix are the same rule.
@@ -5,7 +5,12 @@ use crate::model::cell::{CellKey, CellValue};
use crate::view::Axis;
use crate::workbook::Workbook;
use super::quoting::{pipe_quote, quote_name};
// The one home of the pipe-quoting rule (improvise-i2m5): `improvise-formula`
// owns the grammar that says what a bare name is, and `improv.pest`'s
// `bare_name`/`pipe_quoted` mirror it. A name written here also has to survive
// being read back through `parse_formula`, since `## Formulas` lines carry
// formula text verbatim.
use crate::formula::quote::{pipe_quote, quote_name};
fn format_number(n: f64) -> String {
if n.is_infinite() {
+2 -2
View File
@@ -1,11 +1,11 @@
//! `.improv` persistence: the document lifecycle — save, load, gzip, legacy
//! JSON — over a [`Storage`] backend. The text format itself lives in the
//! sibling modules: `quoting`, `format`, `parse` and `export`.
//! sibling modules: `format`, `parse` and `export`. The pipe-quoting rule
//! both halves share lives in `improvise_formula::quote` (improvise-i2m5).
mod export;
mod format;
mod parse;
mod quoting;
#[cfg(test)]
mod roundtrip_tests;
+324 -255
View File
@@ -1,7 +1,19 @@
//! Parsing the `.improv` text format: a tree-walk over the pest parse tree.
//!
//! The walk is two passes. Pass 1 turns each `## ` section into a plain
//! collector struct ([`PCategory`], [`PView`]) or a flat list; pass 2 builds
//! the [`Workbook`] from those collectors. The split is what makes the format
//! order-independent: a `## View:` section may name a category the file
//! defines further down.
//!
//! One walker per section, each a free function over the section's `Pair`, so
//! a section of the grammar and the code that reads it can be read side by
//! side. The grammar (`improv.pest`) stays the single source of truth for what
//! is accepted; these functions only say what the accepted tree means.
use anyhow::{Context, Result};
use anyhow::{Context, Result, bail};
use pest::Parser;
use pest::iterators::{Pair, Pairs};
use pest_derive::Parser;
use crate::formula::parse_formula;
@@ -10,12 +22,87 @@ use crate::model::cell::{CellKey, CellValue};
use crate::view::Axis;
use crate::workbook::Workbook;
use super::quoting::unescape_pipe;
use crate::formula::quote::unescape_pipe_body;
#[derive(Parser)]
#[grammar = "persistence/improv.pest"]
struct ImprovParser;
// ── Intermediate collectors ──────────────────────────────────────────────────
/// One `## Category:` section as pass 1 read it.
struct PCategory {
name: String,
/// `(item name, owning group)`, in file order — `IndexMap` insertion order
/// is load-bearing, so pass 2 must replay this list as written.
items: Vec<(String, Option<String>)>,
group_parents: Vec<(String, String)>,
}
/// One `## View:` section as pass 1 read it.
struct PView {
name: String,
axes: Vec<(String, Axis)>,
page_selections: Vec<(String, String)>,
format: String,
hidden: Vec<(String, String)>,
collapsed: Vec<(String, String)>,
}
/// Everything pass 1 collects, before any of it is resolved against the model.
#[derive(Default)]
struct ParsedFile {
model_name: Option<String>,
initial_view: Option<String>,
categories: Vec<PCategory>,
formulas: Vec<(String, String)>,
data: Vec<(CellKey, CellValue)>,
views: Vec<PView>,
}
// ── Helpers for walking the pest parse tree ──────────────────────────────────
/// Advance an iterator, returning an error if empty.
fn next<'a>(pairs: &mut Pairs<'a, Rule>, ctx: &str) -> Result<Pair<'a, Rule>> {
pairs
.next()
.ok_or_else(|| anyhow::anyhow!("Expected child in {ctx}"))
}
/// Extract the first child's text content, trimmed.
fn first_str(pair: Pair<'_, Rule>) -> Result<String> {
Ok(next(&mut pair.into_inner(), "first_str")?
.as_str()
.trim()
.to_string())
}
/// A `name` (`pipe_quoted | bare_name`) as the string it denotes.
fn extract_name(pair: Pair<'_, Rule>) -> Result<String> {
match pair.as_rule() {
Rule::bare_name => Ok(pair.as_str().to_string()),
Rule::pipe_quoted => {
let inner = next(&mut pair.into_inner(), "pipe_quoted")?;
Ok(unescape_pipe_body(inner.as_str()))
}
_ => Ok(pair.as_str().to_string()),
}
}
/// Extract two names from a pair's children.
fn extract_name_pair(pair: Pair<'_, Rule>) -> Result<(String, String)> {
let ctx = format!("{:?}", pair.as_rule());
let mut parts = pair.into_inner();
let a = extract_name(next(&mut parts, &ctx)?)?;
let b = extract_name(next(&mut parts, &ctx)?)?;
Ok((a, b))
}
/// The bare section title after `## Category:` / `## View:`, trimmed.
fn section_title(inner: &mut Pairs<'_, Rule>, ctx: &str) -> Result<String> {
Ok(next(inner, ctx)?.as_str().trim().to_string())
}
/// Convert a token matched by the grammar's `number` rule into a cell value.
///
/// The grammar only admits tokens that `f64::from_str` accepts (overflow
@@ -27,15 +114,11 @@ fn parse_number_cell(s: &str) -> CellValue {
.unwrap_or_else(|_| CellValue::Error(format!("invalid number: {s}")))
}
/// Parse the `.improv` format into a Model using the pest grammar.
///
/// Sections may appear in any order; a two-pass approach registers categories
/// before configuring views.
pub fn parse_md(text: &str) -> Result<Workbook> {
use anyhow::bail;
use pest::iterators::{Pair, Pairs};
// ── Pass 1: one walker per section ───────────────────────────────────────────
let file = ImprovParser::parse(Rule::file, text)
/// Run the grammar over `text`, rendering a pest error as a located message.
fn parse_tree(text: &str) -> Result<Pair<'_, Rule>> {
ImprovParser::parse(Rule::file, text)
.map_err(|e| {
use pest::error::LineColLocation;
let (line, col) = match e.line_col {
@@ -48,237 +131,235 @@ pub fn parse_md(text: &str) -> Result<Workbook> {
)
})?
.next()
.ok_or_else(|| anyhow::anyhow!("Empty parse result"))?;
.ok_or_else(|| anyhow::anyhow!("Empty parse result"))
}
// ── Intermediate collectors ──────────────────────────────────────────────
struct PCategory {
name: String,
items: Vec<(String, Option<String>)>,
group_parents: Vec<(String, String)>,
}
struct PView {
name: String,
axes: Vec<(String, Axis)>,
page_selections: Vec<(String, String)>,
format: String,
hidden: Vec<(String, String)>,
collapsed: Vec<(String, String)>,
}
let mut model_name: Option<String> = None;
let mut initial_view: Option<String> = None;
let mut categories: Vec<PCategory> = Vec::new();
let mut formulas: Vec<(String, String)> = Vec::new();
let mut data: Vec<(CellKey, CellValue)> = Vec::new();
let mut views: Vec<PView> = Vec::new();
// ── Helpers for walking the pest parse tree ──────────────────────────────
/// Advance an iterator, returning an error if empty.
fn next<'a>(pairs: &mut Pairs<'a, Rule>, ctx: &str) -> Result<Pair<'a, Rule>> {
pairs
.next()
.ok_or_else(|| anyhow::anyhow!("Expected child in {ctx}"))
}
/// Extract the first child's text content, trimmed.
fn first_str(pair: Pair<'_, Rule>) -> Result<String> {
Ok(next(&mut pair.into_inner(), "first_str")?
.as_str()
.trim()
.to_string())
}
fn extract_name(pair: Pair<'_, Rule>) -> Result<String> {
match pair.as_rule() {
Rule::bare_name => Ok(pair.as_str().to_string()),
Rule::pipe_quoted => {
let inner = next(&mut pair.into_inner(), "pipe_quoted")?;
Ok(unescape_pipe(inner.as_str()))
}
_ => Ok(pair.as_str().to_string()),
}
}
/// Extract two names from a pair's children.
fn extract_name_pair(pair: Pair<'_, Rule>) -> Result<(String, String)> {
let ctx = format!("{:?}", pair.as_rule());
let mut parts = pair.into_inner();
let a = extract_name(next(&mut parts, &ctx)?)?;
let b = extract_name(next(&mut parts, &ctx)?)?;
Ok((a, b))
}
// ── Pass 1: walk the parse tree ─────────────────────────────────────────
for pair in file.into_inner() {
match pair.as_rule() {
Rule::version_line | Rule::EOI => {}
Rule::model_name => {
model_name = Some(first_str(pair)?);
}
Rule::initial_view => {
initial_view = Some(first_str(pair)?);
}
Rule::category_section => {
let mut inner = pair.into_inner();
let cname = next(&mut inner, "category_section")?
.as_str()
.trim()
.to_string();
let mut pc = PCategory {
name: cname,
items: Vec::new(),
group_parents: Vec::new(),
};
for entry in inner {
match entry.as_rule() {
Rule::item_list => {
for name_pair in entry.into_inner() {
pc.items.push((extract_name(name_pair)?, None));
}
}
Rule::grouped_item => {
let (name, group) = extract_name_pair(entry)?;
pc.items.push((name, Some(group)));
}
Rule::group_hierarchy => {
pc.group_parents.push(extract_name_pair(entry)?);
}
_ => {}
}
}
categories.push(pc);
}
Rule::formulas_section => {
for fl in pair.into_inner() {
if fl.as_rule() == Rule::formula_line {
let mut parts = fl.into_inner();
let raw = next(&mut parts, "formula_line")?
.as_str()
.trim()
.to_string();
// No [Category] suffix — default to _Measure
let category = match parts.next() {
Some(name_pair) => extract_name(name_pair)?,
None => "_Measure".to_string(),
};
if raw.contains('=') {
formulas.push((raw, category));
}
}
/// `## Category: Name` plus its item lists, grouped items and `>` hierarchy.
fn walk_category_section(pair: Pair<'_, Rule>) -> Result<PCategory> {
let mut inner = pair.into_inner();
let mut pc = PCategory {
name: section_title(&mut inner, "category_section")?,
items: Vec::new(),
group_parents: Vec::new(),
};
for entry in inner {
match entry.as_rule() {
Rule::item_list => {
for name_pair in entry.into_inner() {
pc.items.push((extract_name(name_pair)?, None));
}
}
Rule::data_section => {
for dl in pair.into_inner() {
if dl.as_rule() == Rule::data_line {
let mut dl_inner = dl.into_inner();
let coord_list = next(&mut dl_inner, "data_line coords")?;
let value_pair = next(&mut dl_inner, "data_line value")?;
let coords: Vec<_> = coord_list
.into_inner()
.filter(|p| p.as_rule() == Rule::coord)
.map(extract_name_pair)
.collect::<Result<_>>()?;
let value = match value_pair.as_rule() {
Rule::number => parse_number_cell(value_pair.as_str()),
Rule::pipe_quoted => {
let inner = next(&mut value_pair.into_inner(), "pipe_quoted")?;
CellValue::Text(unescape_pipe(inner.as_str()))
}
Rule::bare_value => match value_pair.as_str().trim() {
"inf" => CellValue::Number(f64::INFINITY),
"-inf" => CellValue::Number(f64::NEG_INFINITY),
"nan" => CellValue::Number(f64::NAN),
s => CellValue::Text(s.to_string()),
},
_ => CellValue::Text(value_pair.as_str().to_string()),
};
data.push((CellKey::new(coords), value));
}
}
Rule::grouped_item => {
let (name, group) = extract_name_pair(entry)?;
pc.items.push((name, Some(group)));
}
Rule::view_section => {
let mut inner = pair.into_inner();
let vname = next(&mut inner, "view_section")?
.as_str()
.trim()
.to_string();
let mut pv = PView {
name: vname,
axes: Vec::new(),
page_selections: Vec::new(),
format: String::new(),
hidden: Vec::new(),
collapsed: Vec::new(),
};
for entry in inner {
match entry.as_rule() {
Rule::axis_line => {
let mut parts = entry.into_inner();
let cat = extract_name(next(&mut parts, "axis cat")?)?;
let kind_str = next(&mut parts, "axis kind")?.as_str();
let axis = match kind_str {
"row" => Axis::Row,
"column" => Axis::Column,
"page" => Axis::Page,
"filter" => Axis::Filter,
"none" => Axis::None,
_ => bail!("Unknown axis kind: {kind_str}"),
};
pv.axes.push((cat.clone(), axis));
if axis.filters_data()
&& let Some(sel_pair) = parts.next()
{
pv.page_selections.push((cat, extract_name(sel_pair)?));
}
}
Rule::format_line => pv.format = first_str(entry)?,
Rule::hidden_line => pv.hidden.push(extract_name_pair(entry)?),
Rule::collapsed_line => pv.collapsed.push(extract_name_pair(entry)?),
_ => {}
}
}
views.push(pv);
Rule::group_hierarchy => {
pc.group_parents.push(extract_name_pair(entry)?);
}
_ => {}
}
}
Ok(pc)
}
// ── Pass 2: build the Workbook ──────────────────────────────────────────
/// `## Formulas`: `(raw text, target category)` pairs. A line with no
/// `[Category]` suffix targets `_Measure`.
fn walk_formulas_section(pair: Pair<'_, Rule>) -> Result<Vec<(String, String)>> {
let mut out = Vec::new();
for fl in pair.into_inner() {
if fl.as_rule() == Rule::formula_line {
let mut parts = fl.into_inner();
let raw = next(&mut parts, "formula_line")?
.as_str()
.trim()
.to_string();
let category = match parts.next() {
Some(name_pair) => extract_name(name_pair)?,
None => "_Measure".to_string(),
};
if raw.contains('=') {
out.push((raw, category));
}
}
}
Ok(out)
}
let name = model_name.ok_or_else(|| anyhow::anyhow!("Missing model title (# Name)"))?;
/// The right-hand side of a data line.
fn walk_cell_value(value_pair: Pair<'_, Rule>) -> Result<CellValue> {
let value = match value_pair.as_rule() {
Rule::number => parse_number_cell(value_pair.as_str()),
Rule::pipe_quoted => {
let inner = next(&mut value_pair.into_inner(), "pipe_quoted")?;
CellValue::Text(unescape_pipe_body(inner.as_str()))
}
Rule::bare_value => match value_pair.as_str().trim() {
"inf" => CellValue::Number(f64::INFINITY),
"-inf" => CellValue::Number(f64::NEG_INFINITY),
"nan" => CellValue::Number(f64::NAN),
s => CellValue::Text(s.to_string()),
},
_ => CellValue::Text(value_pair.as_str().to_string()),
};
Ok(value)
}
/// `## Data`: `Cat=Item, … = value` lines as keyed cells.
fn walk_data_section(pair: Pair<'_, Rule>) -> Result<Vec<(CellKey, CellValue)>> {
let mut out = Vec::new();
for dl in pair.into_inner() {
if dl.as_rule() == Rule::data_line {
let mut dl_inner = dl.into_inner();
let coord_list = next(&mut dl_inner, "data_line coords")?;
let value_pair = next(&mut dl_inner, "data_line value")?;
let coords: Vec<_> = coord_list
.into_inner()
.filter(|p| p.as_rule() == Rule::coord)
.map(extract_name_pair)
.collect::<Result<_>>()?;
out.push((CellKey::new(coords), walk_cell_value(value_pair)?));
}
}
Ok(out)
}
/// The keyword after `Category:` on an axis line.
fn parse_axis_kind(kind_str: &str) -> Result<Axis> {
match kind_str {
"row" => Ok(Axis::Row),
"column" => Ok(Axis::Column),
"page" => Ok(Axis::Page),
"filter" => Ok(Axis::Filter),
"none" => Ok(Axis::None),
_ => bail!("Unknown axis kind: {kind_str}"),
}
}
/// One axis line: the category, its axis, and — for the axes that constrain
/// data — the selected item that follows the comma.
fn walk_axis_line(entry: Pair<'_, Rule>, pv: &mut PView) -> Result<()> {
let mut parts = entry.into_inner();
let cat = extract_name(next(&mut parts, "axis cat")?)?;
let axis = parse_axis_kind(next(&mut parts, "axis kind")?.as_str())?;
pv.axes.push((cat.clone(), axis));
if axis.filters_data()
&& let Some(sel_pair) = parts.next()
{
pv.page_selections.push((cat, extract_name(sel_pair)?));
}
Ok(())
}
/// `## View: Name` plus its axis, format, hidden and collapsed lines.
fn walk_view_section(pair: Pair<'_, Rule>) -> Result<PView> {
let mut inner = pair.into_inner();
let mut pv = PView {
name: section_title(&mut inner, "view_section")?,
axes: Vec::new(),
page_selections: Vec::new(),
format: String::new(),
hidden: Vec::new(),
collapsed: Vec::new(),
};
for entry in inner {
match entry.as_rule() {
Rule::axis_line => walk_axis_line(entry, &mut pv)?,
Rule::format_line => pv.format = first_str(entry)?,
Rule::hidden_line => pv.hidden.push(extract_name_pair(entry)?),
Rule::collapsed_line => pv.collapsed.push(extract_name_pair(entry)?),
_ => {}
}
}
Ok(pv)
}
/// Walk the whole file, dispatching each top-level section to its walker.
fn walk_file(file: Pair<'_, Rule>) -> Result<ParsedFile> {
let mut parsed = ParsedFile::default();
for pair in file.into_inner() {
match pair.as_rule() {
Rule::version_line | Rule::EOI => {}
Rule::model_name => parsed.model_name = Some(first_str(pair)?),
Rule::initial_view => parsed.initial_view = Some(first_str(pair)?),
Rule::category_section => parsed.categories.push(walk_category_section(pair)?),
Rule::formulas_section => parsed.formulas.extend(walk_formulas_section(pair)?),
Rule::data_section => parsed.data.extend(walk_data_section(pair)?),
Rule::view_section => parsed.views.push(walk_view_section(pair)?),
_ => {}
}
}
Ok(parsed)
}
// ── Pass 2: build the Workbook ───────────────────────────────────────────────
/// Register one category's items and groups, in file order.
fn build_category(wb: &mut Workbook, pc: &PCategory) -> Result<()> {
wb.add_category(&pc.name)?;
let cat = wb
.model
.category_mut(&pc.name)
.ok_or_else(|| anyhow::anyhow!("Category '{}' not found after add", pc.name))?;
for (item_name, group) in &pc.items {
match group {
Some(g) => {
cat.add_item_in_group(item_name, g);
if !cat.groups.iter().any(|e| &e.name == g) {
cat.add_group(Group::new(g));
}
}
None => {
cat.add_item(item_name);
}
}
}
for (group_name, parent) in &pc.group_parents {
match cat.groups.iter_mut().find(|g| &g.name == group_name) {
Some(g) => g.parent = Some(parent.clone()),
None => cat.add_group(Group::new(group_name).with_parent(parent)),
}
}
Ok(())
}
/// Create the view if it is new, then apply its axes, selections and flags.
fn build_view(wb: &mut Workbook, pv: &PView) -> Result<()> {
if !wb.views.contains_key(&pv.name) {
wb.create_view(&pv.name);
}
let view = wb
.views
.get_mut(&pv.name)
.ok_or_else(|| anyhow::anyhow!("View '{}' not found after create", pv.name))?;
for (cat, axis) in &pv.axes {
view.set_axis(cat, *axis);
}
for (cat, sel) in &pv.page_selections {
view.set_page_selection(cat, sel);
}
if !pv.format.is_empty() {
view.number_format = pv.format.clone();
}
for (cat, item) in &pv.hidden {
view.hide_item(cat, item);
}
for (cat, grp) in &pv.collapsed {
view.toggle_group_collapse(cat, grp);
}
Ok(())
}
/// Turn pass 1's collectors into a [`Workbook`].
fn build_workbook(parsed: ParsedFile) -> Result<Workbook> {
let name = parsed
.model_name
.ok_or_else(|| anyhow::anyhow!("Missing model title (# Name)"))?;
let mut wb = Workbook::new(&name);
for pc in &categories {
wb.add_category(&pc.name)?;
let cat = wb
.model
.category_mut(&pc.name)
.ok_or_else(|| anyhow::anyhow!("Category '{}' not found after add", pc.name))?;
for (item_name, group) in &pc.items {
match group {
Some(g) => {
cat.add_item_in_group(item_name, g);
if !cat.groups.iter().any(|e| &e.name == g) {
cat.add_group(Group::new(g));
}
}
None => {
cat.add_item(item_name);
}
}
}
for (group_name, parent) in &pc.group_parents {
match cat.groups.iter_mut().find(|g| &g.name == group_name) {
Some(g) => g.parent = Some(parent.clone()),
None => cat.add_group(Group::new(group_name).with_parent(parent)),
}
}
for pc in &parsed.categories {
build_category(&mut wb, pc)?;
}
// A file written before improvise-60z may still carry the scratch views.
@@ -286,51 +367,39 @@ pub fn parse_md(text: &str) -> Result<Workbook> {
// them here instead of reinstating a view the app would rebuild anyway.
// `initial_view` below then falls through to the default, since the name
// it points at is no longer present.
for pv in views
for pv in parsed
.views
.iter()
.filter(|pv| !crate::view::VIRTUAL_VIEW_NAMES.contains(&pv.name.as_str()))
{
if !wb.views.contains_key(&pv.name) {
wb.create_view(&pv.name);
}
let view = wb
.views
.get_mut(&pv.name)
.ok_or_else(|| anyhow::anyhow!("View '{}' not found after create", pv.name))?;
for (cat, axis) in &pv.axes {
view.set_axis(cat, *axis);
}
for (cat, sel) in &pv.page_selections {
view.set_page_selection(cat, sel);
}
if !pv.format.is_empty() {
view.number_format = pv.format.clone();
}
for (cat, item) in &pv.hidden {
view.hide_item(cat, item);
}
for (cat, grp) in &pv.collapsed {
view.toggle_group_collapse(cat, grp);
}
build_view(&mut wb, pv)?;
}
if let Some(iv) = &initial_view
if let Some(iv) = &parsed.initial_view
&& wb.views.contains_key(iv)
{
wb.active_view = iv.clone();
}
for (raw, cat_name) in &formulas {
for (raw, cat_name) in &parsed.formulas {
wb.model
.add_formula(parse_formula(raw, cat_name).with_context(|| format!("Formula: {raw}"))?);
}
for (key, value) in data {
for (key, value) in parsed.data {
wb.model.set_cell(key, value);
}
Ok(wb)
}
/// Parse the `.improv` format into a Model using the pest grammar.
///
/// Sections may appear in any order; a two-pass approach registers categories
/// before configuring views.
pub fn parse_md(text: &str) -> Result<Workbook> {
build_workbook(walk_file(parse_tree(text)?)?)
}
#[cfg(test)]
mod tests {
use super::parse_md;
@@ -1,63 +0,0 @@
//! Pipe quoting, shared between formatting and parsing.
/// Check whether a name is a valid bare identifier: `[A-Za-z_][A-Za-z0-9_-]*`
pub(super) fn is_bare_name(name: &str) -> bool {
let mut chars = name.chars();
match chars.next() {
Some(c) if c.is_ascii_alphabetic() || c == '_' => {}
_ => return false,
}
chars.all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-')
}
/// Escape a string for use inside pipe delimiters: `\|`, `\\`, `\n`.
pub(super) fn escape_pipe(s: &str) -> String {
let mut out = String::with_capacity(s.len());
for c in s.chars() {
match c {
'|' => out.push_str("\\|"),
'\\' => out.push_str("\\\\"),
'\n' => out.push_str("\\n"),
c => out.push(c),
}
}
out
}
/// Unescape a pipe-quoted body: `\|` → `|`, `\\` → `\`, `\n` → newline.
pub(super) fn unescape_pipe(s: &str) -> String {
let mut out = String::with_capacity(s.len());
let mut chars = s.chars();
while let Some(c) = chars.next() {
if c == '\\' {
match chars.next() {
Some('|') => out.push('|'),
Some('\\') => out.push('\\'),
Some('n') => out.push('\n'),
Some(other) => {
out.push('\\');
out.push(other);
}
None => out.push('\\'),
}
} else {
out.push(c);
}
}
out
}
/// CL-style `|...|` pipe quoting unless the name is a valid bare identifier.
pub(super) fn quote_name(name: &str) -> String {
if is_bare_name(name) {
name.to_string()
} else {
format!("|{}|", escape_pipe(name))
}
}
/// Pipe-quote unconditionally (for text cell values that must be distinguished
/// from numbers).
pub(super) fn pipe_quote(s: &str) -> String {
format!("|{}|", escape_pipe(s))
}
@@ -633,3 +633,94 @@ fn multiple_tricky_text_cells() {
);
}
}
// ── Formula keywords as names (improvise-i2m5) ───────────────────────────
/// `.improv` now spells a name through `improvise_formula::quote_name`, which
/// pipe-quotes a formula keyword. Before the switch, io's own copy of the rule
/// had no keyword guard and wrote `## Category: SUM` / `- WHERE` /
/// `SUM=WHERE = 1` bare; that was the only output difference the whole
/// persistence suite showed.
///
/// The names round-trip either way — `improv.pest` accepts `pipe_quoted`
/// wherever a `name` goes — so this pins the spelling, not the survival.
#[test]
fn a_keyword_named_category_is_pipe_quoted() {
let mut wb = Workbook::new("Budget");
wb.add_category("SUM").unwrap();
wb.add_item("SUM", "WHERE").unwrap();
wb.model
.set_cell(coord(&[("SUM", "WHERE")]), CellValue::Number(1.0));
let text = format_md(&wb);
assert!(text.contains("- |WHERE|"), "{text}");
assert!(text.contains("|SUM|=|WHERE| = 1"), "{text}");
// Section headers are the exception, and were before this change too:
// `category_section` is `"## Category: " ~ rest_of_line`, so the header
// takes the whole line and never goes through `quote_name` on either
// side. Only the *references* to the name are quoted.
assert!(text.contains("## Category: SUM"), "{text}");
}
/// The round trip the quoting exists for: every place a keyword-named
/// category or item reaches the file — the axis line, the page selection,
/// `hidden:`, the category section, the data coordinates — reads back as the
/// name that went in.
#[test]
fn a_keyword_named_category_round_trips_everywhere_it_appears() {
let mut wb = Workbook::new("Budget");
wb.add_category("SUM").unwrap();
wb.add_category("IF").unwrap();
for item in ["WHERE", "COUNT"] {
wb.add_item("SUM", item).unwrap();
}
wb.add_item("IF", "MIN").unwrap();
wb.active_view_mut().set_axis("SUM", Axis::Row);
wb.active_view_mut().set_axis("IF", Axis::Page);
wb.active_view_mut().set_page_selection("IF", "MIN");
wb.active_view_mut().hide_item("SUM", "COUNT");
wb.model.set_cell(
coord(&[("SUM", "WHERE"), ("IF", "MIN")]),
CellValue::Number(7.0),
);
let text = format_md(&wb);
let loaded = parse_md(&text).unwrap();
let cat = loaded.model.category("SUM").expect("category SUM\n{text}");
assert_eq!(cat.ordered_item_names(), vec!["WHERE", "COUNT"], "{text}");
let view = loaded.active_view();
assert_eq!(view.axis_of("SUM"), Axis::Row, "{text}");
assert_eq!(view.axis_of("IF"), Axis::Page, "{text}");
assert_eq!(view.page_selection("IF"), Some("MIN"), "{text}");
assert!(view.is_hidden("SUM", "COUNT"), "{text}");
assert_eq!(
loaded
.model
.get_cell(&coord(&[("SUM", "WHERE"), ("IF", "MIN")])),
Some(&CellValue::Number(7.0)),
"{text}"
);
}
/// Why the keyword guard is worth its cosmetic cost even though bare keywords
/// parse: a `## Formulas` line carries formula text verbatim, so a name
/// written by `format_md` is later read by `parse_formula`. The two grammars
/// share one namespace, and `quote_name` is now the one rule spelling names
/// for both.
#[test]
fn a_formula_targeting_a_keyword_named_measure_round_trips() {
let mut wb = Workbook::new("Budget");
wb.add_category("Region").unwrap();
wb.add_item("Region", "East").unwrap();
wb.model
.add_formula(parse_formula("|COUNT| = |SUM| * 2", "_Measure").unwrap());
let text = format_md(&wb);
let loaded = parse_md(&text).unwrap();
let f = &loaded.model.formulas()[0];
assert_eq!(f.target, "COUNT", "{text}");
assert_eq!(f.target_category, "_Measure", "{text}");
}
+2 -1
View File
@@ -96,10 +96,11 @@ doctrine(characterization_survives_the_move,'for a relocation refactor, write th
doctrine(two_questions_one_enum,'when a new enum variant shares one property with an existing variant but not another, name both properties as predicates (filters_data, is_user_assignable) and route every call site through the one it means; testing for the variant directly is what goes stale when a third variant arrives').
doctrine(contract_test_for_the_unreachable_backend,'when one implementation of a trait cannot be exercised by the test harness, write the contract as a function over &dyn Trait and run it against the reachable implementations; it becomes the executable specification the unreachable one is held to').
doctrine(derive_the_enum_from_the_effects,'when a wire vocabulary must mirror an existing mutation set, enumerate the mutations and let that be the enum; inventing variants ahead of the mutations produces a protocol nobody can implement against').
doctrine(force_the_failure,'an unchecked status whose failure yields a usable-looking value is not a style nit: force the failure and look at what the next line does with it. Four on one branch -- bd init exits non-zero but still leaves an empty database behind, an unchecked mv lands the new tracker INSIDE the old directory and reports success, jq status swallowed by a process substitution reads as \"no inputs found\" and exits 0, and a failed mktemp turns \"$TMP/$NAME\" into an absolute path at the filesystem root. Every one looked like success at runtime, so reading the failure paths found none of them; stubbing mv and mktemp to fail is what exposed them. Note chmod a-w proves nothing when the tests run as root -- it returned 0 and looked like a pass.').
doctrine(force_the_failure,'an unchecked status whose failure yields a usable-looking value is not a style nit: force the failure and look at what the next line does with it. Four on one branch -- bd init exits non-zero but still leaves an empty database behind, an unchecked mv lands the new tracker INSIDE the old directory and reports success, jq status swallowed by a process substitution reads as \"no inputs found\" and exits 0, and a failed mktemp turns \"$TMP/$NAME\" into an absolute path at the filesystem root. Every one looked like success at runtime, so reading the failure paths found none of them; stubbing mv and mktemp to fail is what exposed them. Note chmod a-w proves nothing when the tests run as root -- it returned 0 and looked like a pass. The same shape appears in tooling: requesting a Copilot PR review over REST returns HTTP 200 and silently ignores the reviewer.').
doctrine(kb_is_maintained_through_the_mcp,'prolog-sessions/kb.pl is the MCP serialized clause database, not a hand-written source file. Add facts by loadSession, then loadProgram, then saveSession -- never by appending text to the file. Note loadProgram REPLACES a whole predicate rather than appending to it, so the program you load must carry every existing clause of every predicate it touches, or the rest are silently dropped. Verify counts with findall/length before saving.').
doctrine(fold_tests_must_test_order_not_the_axiom,'a test asserting fold(prefix) then fold(rest) == fold(whole) is VACUOUS: that identity is the definition of a left fold and holds for any apply() whatsoever. Proven 2026-09-19 by rewriting GridCache::apply to write every cell update to (0,0) -- both headline fold tests stayed green while the two real tests failed. Test the properties the design actually rests on: ORDER SENSITIVITY (which updates are dropped when they arrive early, i.e. that the fold is deliberately not commutative) and COMPACTION EQUIVALENCE (fold(compact(history)) == fold(history)), which is what \"seeded, then follows the event stream\" means in production. A seed taken as the literal head of the history presupposes the conclusion.').
doctrine(a_macro_must_not_supply_a_forced_classification,'invariant(effect_kind_no_default) makes the compiler demand an EffectKind per effect precisely because a wrong one is silent. A macro that hard-codes the kind reintroduces the default under another name, for a category defined by SYNTAX (one field, one place) rather than SEMANTICS (per-client). Proven 2026-09-19: a simple_effect! instantiation writing the active view number format -- a textbook LayoutGeometry change -- compiled, reported ViewOnly, and passed all 294 command tests. If a macro covers a classified thing, make the classification a mandatory argument even when the answer is always the same.').
doctrine(one_arm_of_a_symmetric_rule_is_evidence_about_that_arm_only,'when a rule must hold on several arms -- both directions of a fit, every variant that touches one collection, each side of a symmetry -- a test driving ONE arm says nothing about the others, and will sit there green while the rule is violated next door. Worked example 2026-09-19: improvise-rkeq fixed col_widths never being re-fitted when ColLabels changed, and fixed only the label arms; the ColWidths arm, named after the very collection it fits, kept assigning verbatim and put the length mismatch straight back. col_widths_always_span_the_column_labels passed the entire time because it only ever drove the label arm. Copilot caught it on the PR. Enumerate the arms and test each, or assert the rule over a generated set of arms rather than one chosen by hand.').
work_done('improvise-8tk',bug_fix,'ZZ/:wq quit even when the save failed, silently discarding the model. Save now aborts the effect batch on failure; save_to_current_path/1 returns Result<String,String>; three tests in ui/effect.rs.').
work_done('improvise-907',bug_fix,'Import wizard step machine redesign: validation_error() gate at ReviewProposals (no-category and over-limit), history-stack previous() bound to Left on every step, and Done made a real confirmation step holding the built workbook. Superseded improvise-gfq, improvise-o1c, improvise-c6j.').
work_done('improvise-6sq',wont_do,'Merging EditOrDrill and EnterEditAtCursorCmd into one EnterEdit{mode} is not safe: they differ in WHEN the cell value is read, not in a mode parameter. Added ui/app.rs::tests::i_and_o_diverge_on_an_empty_records_grid as executable evidence.').