d7bfcbbd937889cd712767692be9ee22fbeaff72
6 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Edward Langley
|
498582deb1 |
Bound every_model_command() with the derive (improvise-26dm, Codex P1)
The right-hand side of the LayoutGeometry completeness diff was a second hand-written list. Both this branch's own review and ChatGPT Codex found it, and the previous commit corrected the PROSE; Codex asked for the FIX. This is it. the_enumeration_names_every_variant_the_derive_declares diffs every_model_command() against variants::declared::<ModelCommand>() -- serde's derive-generated variant list, improvise-5ubs's mechanism. The chain is now derive -> every_model_command() -> model_effects -> case list, every link checked, which is what the ticket asked for. Injection is Codex's own, run end to end: a ModelCommand::ProbeGeometry whose effect declares LayoutGeometry, with the model_effects and variant_name arms the compiler forces, omitted from BOTH lists. Before: the whole workspace green. After: staged, each stage naming the probe -- every_model_command() never constructs ["ProbeGeometry"] then, with the enumeration repaired and only the case list short, the_layout_geometry_cases_are_every_wire_effect_of_that_kind right: [... "ProbeGeometry" ...] The staging is the point: the two tests answer different questions. SCOPE OVERRUN, declared. `variants` was `#[cfg(test)] pub(crate)`, not merely `pub(crate)`, and `declared()` needs serde_json, which improvise-protocol carries only as a dev-dependency -- so the authorised one-token change could not work. What it took instead, all plumbing, no behaviour change: improvise-protocol/Cargo.toml serde_json optional + a `test-support` feature improvise-protocol/src/lib.rs #[cfg(any(test, feature = "test-support"))] pub improvise-command/Cargo.toml dev-dep enables that feature The feature is OFF in every shipped build: `cargo tree --edges normal --target wasm32-unknown-unknown` shows no serde_json, and all eight wasm-gated crates still check clean. Drop this commit if the crossing is unwanted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
e46429ecfc |
feat(fit): fit-measure — a fitted tree as an ordinary improvise formula
improvise-szrt / improvise-vx3. `improvise-gp` was a bridge with nothing
crossing it; this is the crossing.
`improvise_gp::fit::fit_measure` is the sequence between the two halves that
already existed: `dataset` (a View's records as a feature matrix) and
`fitted_formula` (a champion as formula text the document can resolve). It
lives in `improvise-gp` rather than in the command because none of it is
terminal-specific — a browser fitting the same measure must run exactly these
steps in exactly this order or it gets a different tree from the same seed.
`command/cmd/fit.rs` is the user-facing surface: `fit-measure <target>
[<measure-name>] [<seed>]`, returning `SetStatus` + `MarkDirty` + `AddFormula`.
After it there is nothing GP-specific in the document — the rule is a
`## Formulas` line, recomputed by the fixed-point evaluator, editable in the
formula panel, drillable, saved with no grammar change.
**Where the fit runs, and why.** In `execute`, on the command side. A fit is
decision logic — a pure function of (Model, View, target, seed, FitConfig)
that mutates nothing — so it belongs where decision logic belongs; putting an
expensive stochastic search inside `Effect::apply` would mix it into the one
place the app mutates. That buys purity, not asynchrony, so vx3's AC 4 is met
on its "bounded" arm: `FitRequest::default` is a smaller search than the CLI's,
**measured** at 6ms / 12ms / 105ms over 12 / 200 / 2,000 records in release.
**Three things the sequence had to get right**, each of which fails quietly:
* The fit is standardized and the formula must not be. Written down untouched,
a champion predicts standard deviations — a well-formed formula evaluating to
±5 where the user's data is in the thousands, and nothing downstream can see
it. `denormalize` folds the affine map back into the leaves first.
* The rule is stored as a *new* measure (`<target> (fit)`), never as the target
itself: gotcha 7 makes a formula target and an explicit item one item, so the
cells the fit was learned from would stop being visible.
* `Quality` is an enum. `cv_regress` pools nothing below two folds and scoring
an empty pool gives R² exactly 0 — "nobody checked" reading as "this rule is
worth nothing", which is the loudest thing a fit can say.
**A bug this found, fixed with its own failing test first.** `_Measure` is a
filtering category, and `Workbook::new`'s default view puts it on `Axis::Page`
— so `dataset::filter_coords` pinned it to one measure and the feature matrix
collapsed. On a document whose target is `_Measure`'s first item every feature
column came back blank; otherwise the table was empty and the error blamed the
data ("no visible record has a Revenue value"). `_Measure` is the coordinate
the columns are spread across, so paging it is not "which records am I looking
at", it is "throw away every column". Every existing test in that module built
its view with `View::new` + `on_category_added`, under which an underscore
category lands on `Axis::None` — the fixture was building the one world the
code worked in. The two new regression tests compare row *contents*, because
asserting the header and the row count passes with the bug present.
Evidence, and the faults injected to earn it. Dropping the `denormalize` call
fails `the_fitted_formula_predicts_in_the_documents_own_units` and nothing
else. Both `_Measure`-on-Page tests were watched failing before the one-line
fix. Every new test asserts against a real document — never a helper that
fabricates a `_Measure` item per feature name, which is what let improvise-8xhy
hide. 1216 passing, up 27 from 1189; suite still well under 2s.
Also fixes the refusal message, which a `rustfmt` pass over its multi-line
literal had left with runs of ten spaces mid-sentence — invisible in the source
and to every test that only asked whether a substring was present, and visible
to precisely the user it exists for.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Edward Langley
|
d25f3187d3 |
feat(protocol): extract improvise-protocol crate
The crate the browser epic hangs off: Command (the wire format), ViewState, RenderCache/CellDisplay/Snapshot, and reduce_view. It depends on improvise-core and serde and nothing else -- no IO, no command layer, no terminal -- and improvise-command now depends on it. ViewState, ViewCursor, ViewFrame, AppMode, Panel, MinibufferConfig, MinibufColor, HELP_PAGE_COUNT and the key types move here. Everything that moved is re-exported from ui::app and command::key, so no existing path changed. The three tickets before this one are what made the move mechanical: h0b evicted the unportable fields, a2x added the derives, 8uz took the cursor out of the shared View. Command's client->server half is not invented. It is exactly the set of EffectKind::ViewOnly effects whose target is the portable slice -- h0b had already sorted the portable state from the host-local, so the enumeration was done. Effects touching the wizard, staged drill edits, storage or the layout cache are absent by construction. The other half carries projections: already-formatted display values and their labels. One thing had to be added rather than moved. reduce_view runs on a client that holds no Workbook, and ViewState::cursors is keyed by view name, so ViewState needs its own active_view. That makes two copies of "which view is active"; App::switch_to_view and App::install_workbook are the only writers, and the_two_active_view_names_never_diverge guards it. The 8uz characterization test caught the desync the moment it appeared, because it was calling workbook.switch_view directly. Whether the two are *allowed* to diverge -- two clients on different views -- is improvise-g0f, deliberately left open. It changes the shape of improvise-cqq, and nothing here forecloses either answer. reduce_view has 12 tests including a determinism check: folding the same stream twice gives the same state, which is what stops the server's mirror drifting from the client it mirrors. Every Command round-trips through JSON. The crate is in the CI wasm job. Closes improvise-cqi Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V69PGQfMJuGRmtSWpfVAqn |
||
|
Edward Langley
|
850ea584d1 |
feat(command): make ViewState serializable
ViewState is the wire payload of the Snapshot a newly-connected client receives (improvise-q08), so it and its field types now derive Serialize and Deserialize: AppMode, MinibufferConfig, MinibufColor, ViewFrame, KeyCode, KeyModifiers. serde joins improvise-command's dependencies. The ticket did not anticipate the actual blocker. MinibufferConfig held `buffer_key: &'static str`, and Deserialize cannot produce a borrowed 'static value -- so every text-entry AppMode variant that carries a MinibufferConfig failed, five "lifetime may not live long enough" errors pointing at the variants and none naming the field. buffer_key and TextEntry::buffer are now owned Strings. Cow<'static, str> would keep construction allocation-free but puts container plumbing in a public field. A BufferKey enum would close the name set, but drags the buffers map key type, the script parser's buffer args and the hand-written "search" buffer with it. Mode construction is per-keypress; seven small allocations are not worth designing around. Note this does not reopen improvise-k8h's rejection of a Buffer enum: that was the drift argument -- each side would still name a variant -- which is about who writes the name, not what type it has. buffer_of() still reads it off the mode. k8h doesn't forbid an enum here; scope does. The roundtrip test populates one non-default value per field family, including a payload-carrying mode and an armed prefix; a roundtrip of ViewState::default() would pass while proving nothing. Closes improvise-a2x Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V69PGQfMJuGRmtSWpfVAqn |
||
|
Edward Langley
|
69e77d6823 |
ci: add wasm32 to the toolchain and gate the bundle crates (improvise-ywd)
The audit could not start because the Nix toolchain shipped no wasm32 std and there is no rustup here. Adding targets = ["wasm32-unknown-unknown"] to the flake's rustToolchain was the whole blocker: all four bundle crates — improvise-core, improvise-formula, improvise-io, improvise-command — then check clean on the first try with no feature flags and no source changes. The root improvise crate does not build for wasm because crossterm does not. That is correct: it is the terminal binary, not part of the browser bundle, so the CI job names the four bundle crates rather than checking the workspace. Adds .github/workflows/ci.yml with that wasm job plus a host job running fmt, clippy -D warnings and the test suite — there was no test or lint CI before, only cargo-dist's release workflow. A check gate cannot catch what merely panics at runtime, and the audit's real finding was that two of the ticket's six named hazards are real while four are not. Instant::now() panics on wasm yet compiles, so improvise-command now uses web-time, which re-exports std::time off-wasm. std::fs likewise compiles and then fails in a browser; StartImportWizard and LoadModel still bypass the Storage trait, filed as improvise-fuj because the fix needs a decision on how a wasm build injects its backend. Verified by compilation on both targets and by the native suite. Nothing here has been run on a wasm host. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V69PGQfMJuGRmtSWpfVAqn |
||
|
Edward Langley
|
a85e6ca28c |
refactor(workspace): extract improvise-command crate (improvise-3mm)
Command layer + semantic App state move below the TUI: command/ (cmd,
keymap, parse, key), ui/{app,effect,cat_tree}.rs, and the pure viewport
helpers now live in crates/improvise-command, which depends on core +
formula + io and provably not on ratatui/crossterm (cargo tree clean).
The root crate remains the TUI binary and re-exports improvise_command
under the old crate::command / crate::ui::{app,effect,cat_tree} paths,
so widgets, draw.rs, and main.rs are unchanged. App moves whole (per
the improvise-ltq design note) rather than splitting an AppState out —
its fields are all command-safe after the key/color decoupling.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|