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

Merge commit 'c6de77dbde8d05ba287cb69d92c230089098916f' into wave/km71-edit-formula

Browse Source 
# Conflicts:
#	context/code/command.md
This commit is contained in:
Edward Langley
2026-09-23 08:42:50 -07:00
parent bd49de6b48 c6de77dbde
commit 37a64445e0
27 changed files with 777 additions and 168 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Cargo.lock
Generated
-3
View File
@@ -897,7 +897,6 @@ dependencies = [
"pest_derive",
"pest_meta",
"proptest",
"serde",
]
[[package]]
@@ -997,8 +996,6 @@ checksum = "cc4e190f5d26ca7051642629da2c52fc03bde85a03197c99408dcd291734c855"
dependencies = [
"equivalent",
"hashbrown 0.17.1",
"serde",
"serde_core",
]
[[package]]
Cargo.toml
+1 -1
View File
@@ -45,7 +45,7 @@ serde = { version = "1", features = ["derive"] }
serde_json = { version = "1", features = ["float_roundtrip"] }
anyhow = "1"
thiserror = "2"
indexmap = { version = "2", features = ["serde"] }
indexmap = "2"
chrono = { version = "0.4", features = ["serde"] }
flate2 = "1"
unicode-width = "^0.2"
README.md
+3 -2
View File
@@ -169,8 +169,9 @@ Measure: column
format: ,.2f
```
Gzip-compressed `.improv.gz` is also supported. Legacy JSON is auto-detected
for backward compatibility.
Gzip-compressed `.improv.gz` is also supported. `.improv` text is the only
native document format: the old JSON document format is no longer read, and
opening one fails with an error that says so. JSON *import* is unaffected.
## Import and scripting
context/code/command.md
+49
View File
@@ -3566,3 +3566,52 @@ On success the command's optimistic "Formula updated" stands, except on a
rename, where the effect rewrites the status to name the old and new targets
and any formula that still mentions the old one (`context/code/core.md` §18).
## A wire write reaches only a cell the host projects as a Record (improvise-ciw8)
`App::resolve_record` answers "which record does this surrogate name". Its
fallback, `record_in_layout`, accepts any key the current layout names, and that
includes a collapsed group's fold and a key the None axis aggregates over. It
has to stay that permissive: it is also the basis check's question, and a record
nobody has stored yet (the empty cell a client is about to fill) is found only
there. But resolving is not the same as being writable. A hand-rolled frame
`SetCell { record: RecordHash::of({Month=Q2}), .. }` was answered `Accepted` and
stored a cell at `Month=Q2`, while the host's own projection called that
position `CellId::Aggregate`. A write under a formula (`Derived`) was accepted
the same way.
So `model_effects` asks `App::unwritable` before a wire `SetCell` or `ClearCell`
builds its effect, and refuses through `effect::refuse`. That is the #181 path:
the refusal marks the batch, `Host::mutating_frame` undoes the frame, the asker
gets `Rejected(MutationRefused)`, and the reason is on the host's status line.
The question it asks is the projection's own, never a second derivation of it:
- A cell a formula owns is refused. That is `cell_id`'s own `Derived` rule,
`formula_for(&key)`. It is asked directly because a stored record may sit on
no position in the host's current view.
- A record found **in the store** is a record, whatever view anyone is on.
`RecordHash` is document-scoped (gotcha 44).
- A record found **only through the layout** is asked of `project::cell_id` at
the position of **its cell**: the one whose key is the whole key the write
names, record and measure together. `Aggregate`, a fold or a sum over a
None-axis category, is refused. It must be the whole key, not the first
position carrying the record (improvise-sjdb). `RecordHash` drops `_Measure`,
so every column of a row carries the same record, and with a collapsed
`_Measure` group the first of them can be the fold. That refused the empty
`Units` cell beside a `Money` fold, which the host itself projects as a
`Record`. A write whose whole key the host's grid does not draw is not
refused here, as it was not before this rule existed.
**Why the `vec![]` arm stays.** A reviewer asked whether a `SetCell` whose cell
does not resolve at all, which `model_effects` turns into `vec![]` and so
applies nothing, should also be a `Rejection`. Through a `Host`, it already is,
before that arm is reached. A lone write naming a record that never existed gets
`RecordGone` up front, whether or not the basis claims it, because the create
exemption covers only a *later* command in a frame
(`a_wire_write_to_a_record_that_never_existed_is_rejected`). A later write whose
record nothing in the frame made resolvable gets `ChainBroken` from
`broken_chain`
(`a_frame_whose_write_would_land_nowhere_is_refused_rather_than_dropped`). So
the arm is unreachable from the wire. Making it a refusal would add a second
answer to "does this resolve" beside `broken_chain`, and that answer could only
ever fire after the first had already refused. It is left as the mutation
path's own behaviour for callers that do not go through the session door.
context/code/formula-and-io.md
+40 -2
View File
@@ -471,8 +471,46 @@ document that decompressed perfectly well. Decoding once, afterwards, also
leaves **one** answer to "is this UTF-8" rather than one per branch.
The `.gz` suffix selects gzip regardless of backend: that is a property of the
document name, not of the filesystem (improvise-6mq). Legacy JSON is detected by
a leading `{` and is never written.
document name, not of the filesystem (improvise-6mq).
**`.improv` text is the only native format (improvise-tt22).** A legacy JSON
workbook used to be detected by a leading `{` and loaded read-only through
`Workbook`'s serde derives. That path is gone, on the user's direction on PR #210.
A document that still starts with `{` is refused with an error that names the
format and says it is no longer supported. The test is
`a_legacy_json_workbook_is_refused_by_name`. The error is decided rather than
defaulted, because otherwise `parse_md` would report a grammar error at line 1,
which diagnoses the wrong thing: the file is not malformed `.improv`. The check
is on the decompressed text, so `.improv.gz` is covered too.
The serde derives that existed only for that path went with it: `Workbook`,
`Model`, `View`, `Category`, `Item`, `Group`, `CategoryKind`, `CellKey`,
`DataStore`'s manual impls and the formula AST. So did indexmap's `serde` feature
in `improvise-core`, `improvise-io` and the root crate, and `improvise-formula`'s
serde dependency. Each was removed and the whole workspace rebuilt, so the
compiler named every other consumer. Two remain, and both are the wire:
`Axis` (whose `rename_all = "lowercase"` is its wire spelling) and `PageAxis`.
`CellValue` was never a candidate, since it is carried on the wire too. JSON
*import* (`StartImportWizard`, `ImportJsonHeadless`) is a separate feature and is
untouched.
One test consumer had to move: the effect-classification tests fingerprinted the
model and the views through `serde_json`. They now use `Debug`. `format_md` was
rejected as the replacement, because it skips the virtual views and sorts the
data, so it would have been a weaker fingerprint.
The first version of this paragraph said `Debug` was "no weaker" because it
renders every field, including the derived `formula_cache` the JSON skipped.
**That was wrong (improvise-pc3b).** A derived `Debug` prints a `HashMap` in hash
order, and `recompute_formulas` clears and refills the cache. So a correct
`ViewOnly` effect such as `RebuildLayout` changed the fingerprint in 90 of 200
runs. The suite passed only because the fixture had no formulas, so the cache
was empty. `Model` now has a hand-written `Debug` that renders `formula_cache`
and `measure_agg` in key order. The fingerprint keeps every field, including the
derived one, and a refill in a different hash order is no longer read as a
change. `recomputing_formulas_leaves_the_model_fingerprint_alone` pins this.
`view_only_effects_leave_shared_state_alone` now runs over a formula-bearing
fixture, for the same reason.
`parse_number_cell` returns `CellValue::Error` naming the offending text rather
than a fabricated `0.0`. No token the current grammar admits actually fails
context/code/protocol.md
+1 -1
View File
@@ -863,7 +863,7 @@ others to inherit it works in `cargo test` and nowhere else.
**Why "can parse" and not "parses a frame".** The first version of the rule
covered only the three crates that parse frames and listed them by hand.
Frames are not the only place a user's number crosses a JSON parser.
`improvise-io`'s legacy-JSON `.improv` load, `improvise-command`'s JSON import
`improvise-io`'s legacy-JSON `.improv` load (removed since, improvise-tt22), `improvise-command`'s JSON import
(`StartImportWizard`, `ImportJsonHeadless`) and the root binary's
`improvise import` all parsed with the lossy default, and in the TUI binary
nothing enabled the feature (improvise-6yvg, reproduced: `0.24815714285714285`
context/test-evidence/command.md
+90
View File
@@ -5833,6 +5833,69 @@ on `Food/Jan/Profit`, where there is nothing to clear, and
`Clothing/Jan/Profit = 5` survives. Injected alone into a full workspace run,
the rival fails this test and nothing else.
### a_wire_write_to_a_collapsed_fold_is_rejected_and_stores_nothing
improvise-ciw8, through a real `Host` frame. Q2 is collapsed, and a hand-rolled
client sends `SetCell` (then, separately, `ClearCell`) for `{Month=Q2, _Measure=Cost}`
with a caught-up basis. At `ea119bd9` both were answered `Accepted`, and the
`SetCell` stored `Month=Q2, _Measure=Cost = 7`, the non-item coordinate
improvise-gw18 exists to prevent. The test asserts `Rejected(MutationRefused)`
and an unchanged document for both. Rival: the aggregate refusal disabled. This
test and the None-axis test fail. The control is
`a_wire_write_to_a_stored_cell_beside_a_fold_is_accepted`, so a host that refused
every wire write cannot pass.
### a_wire_write_under_a_formula_is_rejected_and_stores_nothing
improvise-ciw8's `Derived` arm: `SetCell` on Feb × Profit, where
`Profit = Revenue - Cost`. At `ea119bd9` it was accepted and stored a value under
the formula. Rival: the formula refusal disabled. Only this test fails.
### a_wire_write_to_a_none_axis_aggregate_is_rejected_and_stores_nothing
improvise-ciw8's other `Aggregate`. With `Product` on `Axis::None`, the grid's
key `{Region=South, _Measure=Revenue}` is a partial key that no stored record
has, so it resolves only through the layout, where `cell_id` says `Aggregate`.
At `ea119bd9` it was accepted and stored the partial key. It fails under the
same rival as the fold test.
### a_wire_write_to_a_record_that_never_existed_is_rejected
The evidence for keeping `model_effects`' `vec![]` arm rather than turning it
into a refusal (`context/code/command.md`, "A wire write reaches only a cell the
host projects as a Record"). A lone write naming a record that never existed is
answered `RecordGone` before anything runs, both when a caught-up basis claims
it and when a hand-rolled basis omits it. The second half was expected to be
`ChainBroken`, and the test said otherwise: the create exemption covers only
commands after the first in a frame. That surprise is why it is asserted rather
than argued.
### an_empty_cell_beside_a_collapsed_measure_group_is_writable
improvise-sjdb, a regression PR #217 introduced and its review found. `_Measure`
is on Column with Revenue and Cost in a collapsed group `Money` and `Units`
ungrouped, and the Feb row is empty. The host projects Feb × Units as a
`Record`. A caught-up `SetCell` on it was answered `Rejected(MutationRefused)`
at `bc060f67`. It was accepted before #217. `aggregate_refusal` found the
unstored record by `RecordHash` alone, which drops `_Measure`, so it judged the
record at the first column on Feb's row, the `Money` fold, which `cell_id` calls
`Aggregate`. The setup asserts that the fold comes before `Units` on the row and
that the target projects as `Record`, so a fixture where the first column is
harmless cannot pass. The fix looks the cell up by its whole key. The rival is
the old lookup by record, which is this test's red commit, `6e021699`.
### a_stored_record_the_hosts_view_aggregates_is_writable
improvise-sjdb, from the #217 review. South/Revenue is stored, then `Product` is
added on `Axis::None` in the host's view, so the host projects South's position
as `Aggregate`. A stored record is a record wherever it is drawn, because
`RecordHash` is document-scoped (gotcha 44). A records-mode client, or one on
another view, may write it. The setup asserts the host really does project
`Aggregate` there, so the test cannot pass on a view that draws South plainly.
Rival: stored records sent through `cell_id` like unstored ones. That rival
fails this test and nothing else in `improvise-command`. Before this test,
nothing in the suite saw it.
### a_fresh_sessions_slice_is_the_hosts_startup_slice
improvise-v8ab. A fresh session's slice is built twice: `App::with_storage`
@@ -5857,6 +5920,33 @@ that is deliberate. Its helpers seed a client *after* mutating the host (mode,
buffers, cursor), so a `Snapshot` would drop exactly the state those tests
stage. This test is what makes the copy faithful at startup.
### ui::effect::tests::recomputing_formulas_leaves_the_model_fingerprint_alone
improvise-pc3b, found by the review of PR #224 at `e62a289`. That PR moved the
effect-classification fingerprints from `serde_json` to `Debug`. The derived
`Debug` for `Model` prints `formula_cache`, a `HashMap` that `recompute_formulas`
clears and refills, so it prints in hash order. The reviewer measured a correct
`ViewOnly` effect (`RebuildLayout`) changing the fingerprint in 90 of 200 runs
once formulas were present. `test_app` has none, so the suite was green over a
latent flake.
The test builds `formula_app` (two measures across a 2×2 grid, plus
`Profit = Revenue - Cost` and `Margin = Profit / Revenue`) 32 times. Each build
is a fresh `App`, so each gets fresh hash states. Each time, it asserts that
`RebuildLayout` leaves `model_fingerprint` unchanged. One flip in about two makes
32 independent tries fail near-certainly.
**Red before the fix**: the two renderings differed only in `formula_cache`'s
entry order. The fix is a hand-written `Debug` for `Model` that renders
`formula_cache` and `measure_agg` in key order.
**Rival, run:** keep the hand-written `Debug` but print `&self.formula_cache`
unsorted. This test fails. `view_only_effects_leave_shared_state_alone` now runs
over `formula_app` too, so every `ViewOnly` case is checked against a populated
cache. The two earlier injections (a `SetStatus` that writes the model, and one
that writes a view's `number_format`) still fail it.
### a_document_opening_on_a_records_view_opens_the_host_in_records_normal
improvise-y5eo. `App::with_storage` set `view_state.mode` to `Normal` and never
context/test-evidence/core.md
+27 -6
View File
@@ -476,12 +476,6 @@ cell in the row).
The pair `live_records` yields is not two facts that could drift apart: the
surrogate is the record's own, so it is the one that resolves it back.
### a_deserialized_store_can_resolve_its_records
The record index is derived state and is `#[serde(skip)]`-equivalent — it must
come back *with* the data, rebuilt through `set`, rather than being written
alongside it.
## `model/record.rs`
### a_records_surrogate_is_pinned_to_a_literal
@@ -1481,6 +1475,33 @@ because the depth bound the fold path used before is gone. Without the marker
the evaluation recurses until the stack runs out, which is why the marker is
there rather than being an optimisation.
### one_formula_at_two_fold_keys_is_evaluated_at_each
improvise-d7aj, found by the adversarial re-review of PR #203 at `b3379d1a`. The
fold memo in `fold_formula_once` is keyed by the whole `CellKey`, and nothing
pinned that: both tests above keep one key context throughout, so a memo keyed
by the formula alone passed the whole workspace. A reference to an item of
**another** category moves the key while `_Measure` names the same formula.
Stores A (Jan 3, Feb 4) and B (Jan 100, Feb 200), `G = Revenue * 2`,
`F = IF(Revenue > 50, G, G + B)`. At `(Q1, A, F)` Revenue is 7, so F is
`G + B`: G at A is 14, and `B` reads F at `(Q1, B)`, where Revenue is 300, so
that F is G at B, 600. The answer is **614**. Rival: the memo keyed by
`formula.target` alone. F is still in progress at `(Q1, A)` when F at `(Q1, B)`
is asked, so that key reads `circular`. Injected into a full workspace run, it
fails this test and nothing else. The reviewer's probe, now committed.
### a_diamond_at_a_fold_is_not_circular
improvise-d7aj's second case, which passed at `b3379d1a` and was pinned by
nothing. `A1 = B2 + C3`, `B2 = D * 2`, `C3 = D * 3`, `D = Revenue` reaches D by
two paths in one fold evaluation, so the second reach must find D *finished*
and not *in progress*. It asserts **35** over D = 7 and exactly four
evaluations, so a memo that re-evaluates D passes neither. Rival: never store a
finished value, so only the in-progress marker stays. This test fails with
`circular`, and so does `a_fold_evaluates_each_formula_it_reaches_once`, whose
doubling chain is also a diamond at every step. So this test is a named pin of
that shape rather than the only guard against it.
### a_fold_answers_is_and_where_on_its_category_as_the_none_axis_does
improvise-lnkb, ruled by the orchestrator: on a fold, the folded category reads
context/test-evidence/formula-and-io.md
+28 -10
View File
@@ -1435,16 +1435,34 @@ reintroduce the iot6 divergence between what the user sees and what they export.
On the pre-6qet tree the body is empty (the group had no row, and pruning left
nothing), so the test fails there.
## Legacy JSON numbers (improvise-6yvg)
## Legacy JSON documents (improvise-6yvg, improvise-tt22)
### `a_legacy_json_workbook_loads_its_numbers_bit_for_bit`
`a_legacy_json_workbook_loads_its_numbers_bit_for_bit` (improvise-6yvg) pinned
that the legacy JSON load path read numbers exactly. improvise-tt22 removed that
path, so the test went with it. Its other role was to witness `improvise-io`'s own
`float_roundtrip` declaration. The manifest rule in
`every_crate_that_can_parse_json_enables_float_roundtrip` (`tests/json_numbers.rs`)
still covers that, because `improvise-io` keeps a normal `serde_json`
dependency for its import types (`context/test-evidence/tooling.md`).
A workbook with Food/Jan holding `0.24815714285714285` is serialized with
`serde_json` and loaded through `load_from`. A leading `{` sends it down the
legacy JSON path. The test first asserts that the text holds the exact decimal,
so what it measures is the parse. It compares `f64::to_bits`. Found by the
review of PR #190, not by the ticket.
### `a_legacy_json_workbook_is_refused_by_name`
**Red at `1158c23`** in `cargo test -p improvise-io`: loaded
`0.24815714285714283`. Green once `improvise-io`'s own manifest enables
`float_roundtrip`, and red again with only that line reverted.
improvise-tt22, on the user's direction on PR #210: `.improv` text is the only
native format. A document starting with `{` used to be deserialized as a
`Workbook`. The test writes a JSON workbook in the shape the old loader read,
both plain and gzipped, and asserts the error names "legacy JSON" and says "no
longer supported".
**Red at `52c6a05`**: `Failed to deserialize workbook: missing field
`next_category_id``. That is a parse error, which is exactly what the ticket said
the refusal must not be. A complete legacy document instead loaded successfully,
which the since-deleted bit-for-bit test above demonstrated.
**Rivals, each run:**
- **Deleting the refusal.** The text falls through to `parse_md`, and the test
fails on `legacy.improv` with "Parse error at line 1, column 1 (expected
version_line)". A parse error does not name the format.
- **Refusing only when the path is not gzipped.** The plain case passes and the
test fails on `legacy.improv.gz` with the same grammar error. So the gzip
case is what makes the check prove it runs on decompressed text.
context/test-evidence/tooling.md
+3 -2
View File
@@ -696,8 +696,9 @@ every target-specific entry, whose **normal** serde_json dependency lacks
**Faults injected, each run:**
- **The feature removed from `improvise-io` alone.** The check fails naming
`improvise-io (all targets)`, and so does
`a_legacy_json_workbook_loads_its_numbers_bit_for_bit`.
`improvise-io (all targets)`. So did
`a_legacy_json_workbook_loads_its_numbers_bit_for_bit`, until improvise-tt22
removed the legacy load path and that test with it.
`a_json_import_stores_its_numbers_bit_for_bit` stays green, because
`improvise-command` enables the feature and unification reaches io in its
build.
context/test-evidence/tui.md
+25 -3
View File
@@ -428,9 +428,31 @@ inputs. This test is aimed at exactly that gap.
**It is deliberately a directory walk rather than a list of names**: a list is a
second place to remember, and the file that gets forgotten is the one nobody
added to it. The scope is the two directories `.gitignore` un-ignores —
`examples/` and `web/`; everything else matching `*.improv` is somebody's
working file.
added to it. The scope is the directories `.gitignore` un-ignores `*.improv`
in. Everything else matching `*.improv` is somebody's working file.
**The directory list is read from `.gitignore`, not written in the test
(improvise-qlrh).** The test used to hard-code `["examples", "web"]`, a list of
exactly the kind the paragraph above warns against. When improvise-plcx
un-ignored `harness/fixtures/*.improv`, nothing added that directory to the
walk. Its two fixtures were then loaded only by the Playwright job, where a
missing seed degrades to an empty model and surfaces as a missing-label
failure, not a load error. `shipped_directories` now takes every
`!<dir>/*.improv` line, so un-ignoring a directory is what enlists it.
The precondition is per directory: each derived directory must yield at least
one document. The old whole-walk `>= 2` could not see a directory silently lost.
**Red, measured at `705c92d`:** `harness/fixtures/tall.improv` was replaced with
`{"legacy": true}`, and the old test still passed. **Rivals, each run against
the new test:**
- **That same broken fixture.** Fails with "…/harness/fixtures/tall.improv does
not load".
- **A walk that drops every `harness` path.** Fails the per-directory
precondition, naming `harness/fixtures`.
- **A derivation that matches no `.gitignore` line.** Fails "…must un-ignore at
least one *.improv directory".
**It matches on the full name, not on `Path::extension()`**: `.improv.gz` is the
same document gzipped and `extension()` calls it `gz`, so matching on the
crates/improvise-command/src/ui/app.rs
+43 -3
View File
@@ -27,7 +27,7 @@ use crate::ui::viewport::{compute_col_widths, compute_row_header_width, compute_
use crate::view::{GridLayout, View};
use crate::workbook::Workbook;
use improvise_core::model::record::{AggregateHash, RecordHash};
use improvise_protocol::RecordCell;
use improvise_protocol::{CellId, RecordCell};
/// A cell's coordinate tuple with `_Measure` dropped: the unit of atomicity
/// ADR 001 settles on, and not the cell — see `context/code/command.md`.
@@ -576,16 +576,56 @@ impl App {
.map(|key| record_of(&key))
}
/// Why a wire write to `cell` is refused: the host itself projects it as no
/// `CellId::Record` (improvise-ciw8, ADR 002). `None` for a writable cell.
pub fn unwritable(&self, cell: &RecordCell) -> Option<String> {
let key = self.resolve_cell(cell)?;
let model = &self.model_state.workbook.model;
let derived = model.formula_for(&key).map(|f| {
format!(
"{} is computed by a formula — edit the formula, not the cell",
f.target
)
});
derived.or_else(|| self.aggregate_refusal(cell.record, &key))
}
/// A record the store does not hold is known only by its layout position, and
/// is a record only where the projection at ITS CELL says so (improvise-sjdb).
fn aggregate_refusal(&self, hash: RecordHash, key: &CellKey) -> Option<String> {
let stored = self.model_state.workbook.model.data.resolve_record(hash);
let (row, col) = stored.map_or_else(|| self.cell_position(key), |_| None)?;
let projected =
crate::ui::project::cell_id(&self.model_state.workbook.model, &self.layout, row, col);
matches!(projected, Some(CellId::Aggregate(_))).then(|| {
"an aggregate combines several records, so there is no one cell to write".into()
})
}
/// Where in the current layout the cell `key` sits, if anywhere. By the whole
/// key: a `RecordHash` drops `_Measure`, so a row's columns all share one.
fn cell_position(&self, key: &CellKey) -> Option<(usize, usize)> {
self.layout_cells()
.find(|(_, _, at)| at == key)
.map(|(row, col, _)| (row, col))
}
/// Every cell key the current grid names. Records mode is excluded: its
/// `cell_key` carries *positions*, the trap ADR 002 rejected `_Index` for.
fn layout_cell_keys(&self) -> impl Iterator<Item = CellKey> + '_ {
self.layout_cells().map(|(_, _, key)| key)
}
/// [`Self::layout_cell_keys`] with each key's position.
fn layout_cells(&self) -> impl Iterator<Item = (usize, usize, CellKey)> + '_ {
let layout = &self.layout;
let rows = match layout.is_records_mode() {
true => 0,
false => layout.row_count(),
};
(0..rows)
.flat_map(move |r| (0..layout.col_count()).filter_map(move |c| layout.cell_key(r, c)))
(0..rows).flat_map(move |r| {
(0..layout.col_count()).filter_map(move |c| layout.cell_key(r, c).map(|k| (r, c, k)))
})
}
/// Point **this client's** slice at `name`, if the document has such a view,
crates/improvise-command/src/ui/effect.rs
+58 -6
View File
@@ -1458,16 +1458,67 @@ mod tests {
// ── Effect classification (improvise-mae) ───────────────────────────
/// Serialized model data — everything a connected client renders.
/// Excludes the view slice, which is per-client.
/// Every field of the model, rendered — everything a connected client
/// renders. `Debug`, not serde: the model has no serde form (improvise-tt22).
fn model_fingerprint(app: &App) -> String {
serde_json::to_string(&app.model_state.workbook.model).expect("model serializes")
format!("{:?}", app.model_state.workbook.model)
}
/// Serialized view configuration. Kept for the NEGATIVE arm only, where a
/// Every field of every view. Kept for the NEGATIVE arm only, where a
/// hash stricter than `document_config` is wanted (improvise-1xf4).
fn view_config_fingerprint(app: &App) -> String {
serde_json::to_string(&app.model_state.workbook.views).expect("views serialize")
format!("{:?}", app.model_state.workbook.views)
}
/// `test_app` plus measures, data and two chained formulas, so the derived
/// formula cache holds several entries for a recompute to refill.
fn formula_app() -> App {
let mut app = test_app();
let wb = &mut app.model_state.workbook;
for m in ["Revenue", "Cost"] {
wb.add_item("_Measure", m).unwrap();
}
let mut n = 1.0;
for t in ["Food", "Clothing"] {
for mo in ["Jan", "Feb"] {
for (m, scale) in [("Revenue", 10.0), ("Cost", 3.0)] {
let key = CellKey::new(vec![
("Type".into(), t.into()),
("Month".into(), mo.into()),
("_Measure".into(), m.into()),
]);
wb.model.set_cell(key, CellValue::Number(n * scale));
}
n += 1.0;
}
}
for raw in ["Profit = Revenue - Cost", "Margin = Profit / Revenue"] {
AddFormula {
raw: raw.into(),
target_category: "_Measure".into(),
}
.apply(&mut app);
}
app.rebuild_layout();
app
}
/// improvise-pc3b: the fingerprint must not read a derived cache's hash
/// order. Evidence: `context/test-evidence/command.md`.
#[test]
fn recomputing_formulas_leaves_the_model_fingerprint_alone() {
// Each App gets fresh hash states; one run observed ~45% flips, so 32
// independent tries make a hash-ordered fingerprint fail near-certainly.
for _ in 0..32 {
let mut app = formula_app();
let before = model_fingerprint(&app);
RebuildLayout.apply(&mut app);
assert_eq!(
model_fingerprint(&app),
before,
"a ViewOnly recompute changed the model fingerprint"
);
}
}
/// improvise-mae: rather than restate each declaration, apply the effect and
@@ -1507,7 +1558,8 @@ mod tests {
EffectKind::ViewOnly,
"{effect:?} is in the ViewOnly case list"
);
let mut app = test_app();
// With formulas, so `RebuildLayout` refills a non-empty cache (pc3b).
let mut app = formula_app();
let model_before = model_fingerprint(&app);
let views_before = view_config_fingerprint(&app);
effect.apply(&mut app);
crates/improvise-command/src/ui/host.rs
+254
View File
@@ -1358,6 +1358,260 @@ mod tests {
);
}
fn month_cell(month: &str, measure: &str) -> CellKey {
CellKey::new(vec![
("Month".to_string(), month.to_string()),
("_Measure".to_string(), measure.to_string()),
])
}
/// Months in two quarters on Row, Revenue/Cost and `Profit = Revenue - Cost` on
/// Column, every month funded, Q2 collapsed: rows `[Jan, Feb, Q2]`.
fn host_on_a_fold() -> Host {
let mut wb = Workbook::new("T");
wb.add_category("Month").unwrap();
for (month, quarter) in [("Jan", "Q1"), ("Feb", "Q1"), ("Apr", "Q2"), ("May", "Q2")] {
wb.add_item_in_group("Month", month, quarter).unwrap();
for measure in ["Revenue", "Cost"] {
wb.add_item("_Measure", measure).unwrap();
let value = CellValue::Number(10.0);
wb.model.set_cell(month_cell(month, measure), value);
}
}
let profit = crate::formula::parse_formula("Profit = Revenue - Cost", "_Measure");
wb.model.add_formula(profit.unwrap());
wb.active_view_mut().set_axis("Month", Axis::Row);
wb.active_view_mut().set_axis("_Measure", Axis::Column);
wb.active_view_mut().toggle_group_collapse("Month", "Q2");
let mut app = App::new(wb, None);
app.rebuild_layout();
assert!(
app.layout.folds_a_group(2, 1),
"setup: row 2 is the Q2 fold"
);
Host::new(app)
}
/// One wire write, as a hand-rolled client composes it: a surrogate built from
/// a key, and a basis claiming whatever the host says is current.
fn wire_write(host: &mut Host, key: &CellKey, clear: bool) -> Vec<Addressed> {
use improvise_protocol::ModelCommand;
let cell = RecordCell {
record: RecordHash::of(key),
measure: key.get("_Measure").map(String::from),
};
let cmd = match clear {
true => ModelCommand::ClearCell(cell),
false => ModelCommand::SetCell {
cell,
value: CellValue::Number(7.0),
},
};
let (asker, _) = host.connect(10, 10);
let frame = crate::ui::reduce::caught_up(host.app(), cmd);
host.handle(asker, ClientFrame::Commands(vec![frame]))
}
fn refused(answer: &[Addressed]) -> bool {
answer
.iter()
.any(|a| matches!(a.msg, ServerFrame::Rejected(Rejection::MutationRefused)))
}
fn document_text(host: &Host) -> String {
crate::persistence::format_md(&host.app().model_state.workbook)
}
/// improvise-ciw8 — see context/test-evidence/command.md.
#[test]
fn a_wire_write_to_a_collapsed_fold_is_rejected_and_stores_nothing() {
for clear in [false, true] {
let mut host = host_on_a_fold();
let before = document_text(&host);
let answer = wire_write(&mut host, &month_cell("Q2", "Cost"), clear);
assert!(refused(&answer), "clear={clear}: not refused: {answer:?}");
assert_eq!(
document_text(&host),
before,
"clear={clear}: the fold was written"
);
}
}
/// improvise-ciw8: a formula's result is `CellId::Derived`, no write's target.
#[test]
fn a_wire_write_under_a_formula_is_rejected_and_stores_nothing() {
let mut host = host_on_a_fold();
let before = document_text(&host);
let answer = wire_write(&mut host, &month_cell("Feb", "Profit"), false);
assert!(refused(&answer), "not refused: {answer:?}");
assert_eq!(
document_text(&host),
before,
"a value was stored under the formula"
);
}
/// improvise-ciw8: a key the None axis aggregates over is `CellId::Aggregate`.
#[test]
fn a_wire_write_to_a_none_axis_aggregate_is_rejected_and_stores_nothing() {
let mut host = host();
host.app
.model_state
.workbook
.add_category("Product")
.unwrap();
host.app
.model_state
.workbook
.add_item("Product", "Widget")
.unwrap();
let view = host.app.model_state.workbook.active_view_mut();
view.set_axis("Product", Axis::None);
host.app.rebuild_layout();
let before = document_text(&host);
let answer = wire_write(&mut host, &cell("South"), false);
assert!(refused(&answer), "not refused: {answer:?}");
assert_eq!(document_text(&host), before, "the partial key was written");
}
/// improvise-ciw8's second question: a write naming a record that never existed
/// is refused by the frame, not dropped — see context/test-evidence/command.md.
#[test]
fn a_wire_write_to_a_record_that_never_existed_is_rejected() {
use improvise_protocol::ModelCommand;
let mut host = host_on_a_fold();
let before = document_text(&host);
// Claimed by a caught-up basis: structural staleness, answered up front.
let answer = wire_write(&mut host, &month_cell("Nowhere", "Cost"), false);
assert!(
answer
.iter()
.any(|a| matches!(a.msg, ServerFrame::Rejected(Rejection::RecordGone(_)))),
"claimed: {answer:?}"
);
// Unclaimed: the create exemption covers only LATER commands in a frame,
// so a lone write is answered up front too.
let key = month_cell("Nowhere", "Cost");
let cmd = ModelCommand::SetCell {
cell: RecordCell {
record: RecordHash::of(&key),
measure: Some("Cost".into()),
},
value: CellValue::Number(7.0),
};
let basis = improvise_protocol::Basis {
document_generation: host.app().document_generation(),
layout_version: host.app().layout_version(),
records: vec![],
};
let (asker, _) = host.connect(10, 10);
let frame = ClientFrame::Commands(vec![Command::Model { basis, cmd }]);
let answer = host.handle(asker, frame);
assert!(
answer
.iter()
.any(|a| matches!(a.msg, ServerFrame::Rejected(Rejection::RecordGone(_)))),
"unclaimed: {answer:?}"
);
assert_eq!(document_text(&host), before);
}
/// The control for the three above: the stored cell beside the fold is written.
#[test]
fn a_wire_write_to_a_stored_cell_beside_a_fold_is_accepted() {
let mut host = host_on_a_fold();
let answer = wire_write(&mut host, &month_cell("Feb", "Cost"), false);
assert!(
answer
.iter()
.any(|a| matches!(a.msg, ServerFrame::Accepted)),
"{answer:?}"
);
let model = &host.app().model_state.workbook.model;
let value = model.get_cell(&month_cell("Feb", "Cost"));
assert_eq!(value, Some(&CellValue::Number(7.0)));
}
/// improvise-sjdb: an empty record is judged at its own cell, not at the first
/// column carrying it — see context/test-evidence/command.md.
#[test]
fn an_empty_cell_beside_a_collapsed_measure_group_is_writable() {
let mut wb = Workbook::new("T");
wb.add_category("Month").unwrap();
for month in ["Jan", "Feb"] {
wb.add_item("Month", month).unwrap();
}
for measure in ["Revenue", "Cost"] {
wb.add_item_in_group("_Measure", measure, "Money").unwrap();
}
wb.add_item("_Measure", "Units").unwrap();
let jan = month_cell("Jan", "Revenue");
wb.model.set_cell(jan, CellValue::Number(10.0));
wb.active_view_mut().set_axis("Month", Axis::Row);
wb.active_view_mut().set_axis("_Measure", Axis::Column);
wb.active_view_mut()
.toggle_group_collapse("_Measure", "Money");
let mut app = App::new(wb, None);
app.rebuild_layout();
let target = month_cell("Feb", "Units");
let units = (0..app.layout.col_count())
.find(|&c| app.layout.cell_key(1, c).as_ref() == Some(&target))
.expect("setup: Feb x Units is drawn");
let fold = (0..app.layout.col_count())
.find(|&c| app.layout.folds_a_group(1, c))
.expect("setup: a folded measure column");
assert!(fold < units, "setup: the fold comes first on Feb's row");
let id =
crate::ui::project::cell_id(&app.model_state.workbook.model, &app.layout, 1, units);
let projected = matches!(id, Some(improvise_protocol::CellId::Record(_)));
assert!(projected, "setup: Feb x Units projects as {id:?}");
let mut host = Host::new(app);
let answer = wire_write(&mut host, &target, false);
let accepted = answer
.iter()
.any(|a| matches!(a.msg, ServerFrame::Accepted));
assert!(accepted, "a projected Record was refused: {answer:?}");
let model = &host.app().model_state.workbook.model;
assert_eq!(model.get_cell(&target), Some(&CellValue::Number(7.0)));
}
/// A STORED record is writable whatever the host's view does to it — see
/// context/test-evidence/command.md (improvise-sjdb).
#[test]
fn a_stored_record_the_hosts_view_aggregates_is_writable() {
let mut wb = workbook();
wb.model.set_cell(cell("South"), CellValue::Number(1.0));
wb.add_category("Product").unwrap();
wb.add_item("Product", "Widget").unwrap();
wb.active_view_mut().set_axis("Product", Axis::None);
let mut app = App::new(wb, None);
app.rebuild_layout();
let south = (0..app.layout.row_count())
.flat_map(|r| (0..app.layout.col_count()).map(move |c| (r, c)))
.find(|&(r, c)| app.layout.cell_key(r, c).as_ref() == Some(&cell("South")))
.expect("setup: South's cell is drawn");
let id = crate::ui::project::cell_id(
&app.model_state.workbook.model,
&app.layout,
south.0,
south.1,
);
let aggregate = matches!(id, Some(improvise_protocol::CellId::Aggregate(_)));
assert!(aggregate, "setup: the host's view aggregates South: {id:?}");
let mut host = Host::new(app);
let answer = wire_write(&mut host, &cell("South"), false);
let accepted = answer
.iter()
.any(|a| matches!(a.msg, ServerFrame::Accepted));
assert!(accepted, "a stored record was refused: {answer:?}");
let model = &host.app().model_state.workbook.model;
assert_eq!(
model.get_cell(&cell("South")),
Some(&CellValue::Number(7.0))
);
}
/// Clause 4 must not fire on a frame that merely names a record it is
/// creating. See `context/test-evidence/command.md`.
#[test]
crates/improvise-command/src/ui/reduce.rs
+12 -10
View File
@@ -135,18 +135,20 @@ pub fn apply_command(app: &mut App, cmd: Checked<'_>) -> Dirty {
/// was derived from (improvise-kmgs). Exhaustive on purpose.
pub fn model_effects(app: &App, cmd: &ModelCommand) -> Vec<Box<dyn Effect>> {
let one: Box<dyn Effect> = match cmd {
// A cell the host projects as no `Record` is refused, so the frame is
// Rejected rather than written (improvise-ciw8, ADR 002).
ModelCommand::SetCell { cell, value } => {
let Some(key) = app.resolve_cell(cell) else {
return vec![];
};
Box::new(effect::SetCell(key, value.clone()))
}
ModelCommand::ClearCell(cell) => {
let Some(key) = app.resolve_cell(cell) else {
return vec![];
};
Box::new(effect::ClearCell(key))
match (app.unwritable(cell), app.resolve_cell(cell)) {
(Some(why), _) => effect::refuse(why),
(None, None) => return vec![],
(None, Some(key)) => Box::new(effect::SetCell(key, value.clone())),
}
}
ModelCommand::ClearCell(cell) => match (app.unwritable(cell), app.resolve_cell(cell)) {
(Some(why), _) => effect::refuse(why),
(None, None) => return vec![],
(None, Some(key)) => Box::new(effect::ClearCell(key)),
},
ModelCommand::AddItem { category, item } => Box::new(effect::AddItem {
category: category.clone(),
item: item.clone(),
crates/improvise-core/Cargo.toml
+1 -1
View File
@@ -9,7 +9,7 @@ repository = "https://github.com/fiddlerwoaroof/improvise"
[dependencies]
improvise-formula = { path = "../improvise-formula" }
anyhow = "1"
indexmap = { version = "2", features = ["serde"] }
indexmap = "2"
serde = { version = "1", features = ["derive"] }
# SipHash-2-4 under a constant key: the record surrogate must not move
# between processes, which rules out anything built on `RandomState`
crates/improvise-core/src/model/category.rs
+4 -6
View File
@@ -1,10 +1,9 @@
use indexmap::IndexMap;
use serde::{Deserialize, Serialize};
pub type CategoryId = usize;
pub type ItemId = usize;
#[derive(Debug, Clone, Serialize, Deserialize)]
#[derive(Debug, Clone)]
pub struct Item {
pub id: ItemId,
pub name: String,
@@ -27,7 +26,7 @@ impl Item {
}
}
#[derive(Debug, Clone, Serialize, Deserialize)]
#[derive(Debug, Clone)]
pub struct Group {
pub name: String,
/// Parent group name for nested hierarchies
@@ -50,7 +49,7 @@ impl Group {
/// Regular categories store their items explicitly; virtual ones are
/// synthesized at query time by the layout layer. context/code/core.md §9.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize, Default)]
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Default)]
pub enum CategoryKind {
#[default]
Regular,
@@ -125,7 +124,7 @@ impl CategoryKind {
}
}
#[derive(Debug, Clone, Serialize, Deserialize)]
#[derive(Debug, Clone)]
pub struct Category {
pub id: CategoryId,
pub name: String,
@@ -136,7 +135,6 @@ pub struct Category {
/// Next item id counter
next_item_id: ItemId,
/// Whether this is a regular or virtual category
#[serde(default)]
pub kind: CategoryKind,
}
crates/improvise-core/src/model/cell.rs
+1 -37
View File
@@ -7,7 +7,7 @@ use super::symbol::{Symbol, SymbolTable};
/// A cell key: (category, item) pairs, always sorted by category name. The
/// constructors are the only way to build one — context/code/core.md §1.
#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct CellKey(pub Vec<(String, String)>);
impl CellKey {
@@ -194,8 +194,6 @@ impl RecordEntry {
}
}
/// Serialized as a list of (key, value) pairs, so `CellKey` need not
/// serialize as a string for JSON object keys.
#[derive(Debug, Clone, Default)]
pub struct DataStore {
/// Primary storage: interned keys, insertion-ordered so records mode can
@@ -210,29 +208,6 @@ pub struct DataStore {
records: HashMap<RecordHash, RecordEntry>,
}
impl Serialize for DataStore {
fn serialize<S: serde::Serializer>(&self, s: S) -> Result<S::Ok, S::Error> {
use serde::ser::SerializeSeq;
let mut seq = s.serialize_seq(Some(self.cells.len()))?;
for (k, v) in &self.cells {
let cell_key = self.to_cell_key(k);
seq.serialize_element(&(cell_key, v))?;
}
seq.end()
}
}
impl<'de> Deserialize<'de> for DataStore {
fn deserialize<D: serde::Deserializer<'de>>(d: D) -> Result<Self, D::Error> {
let pairs: Vec<(CellKey, CellValue)> = Vec::deserialize(d)?;
let mut store = DataStore::default();
for (key, value) in pairs {
store.set(key, value);
}
Ok(store)
}
}
impl DataStore {
pub fn new() -> Self {
Self::default()
@@ -1181,15 +1156,4 @@ mod record_index {
assert_eq!(store.resolve_record(hash), Some(record));
}
}
/// Derived state must come back *with* the data, rebuilt through `set`,
/// rather than be written alongside it.
#[test]
fn a_deserialized_store_can_resolve_its_records() {
let json = serde_json::to_string(&store()).expect("serializes");
let back: DataStore = serde_json::from_str(&json).expect("deserializes");
let east = key(&[("Product", "Shirts"), ("Region", "East")]);
assert_eq!(back.resolve_record(RecordHash::of(&east)), Some(&east));
assert_eq!(back.live_records().count(), 2);
}
}
crates/improvise-core/src/model/types.rs
+58 -4
View File
@@ -2,7 +2,6 @@ use std::collections::HashMap;
use anyhow::{Result, anyhow};
use indexmap::{IndexMap, IndexSet};
use serde::{Deserialize, Serialize};
use super::category::{Category, CategoryId, CategoryKind, MEASURE_CATEGORY};
use super::cell::{CellKey, CellValue, DataStore};
@@ -300,7 +299,7 @@ impl Aggregation {
/// Pure-data document model: categories, cells, formulas. It knows nothing
/// about views — see [`crate::workbook::Workbook`] and core.md §10.
#[derive(Debug, Clone, Serialize, Deserialize)]
#[derive(Clone)]
pub struct Model {
pub name: String,
pub categories: IndexMap<String, Category>,
@@ -309,13 +308,31 @@ pub struct Model {
next_category_id: CategoryId,
/// Per-measure aggregation, used when collapsing `Axis::None`
/// categories. Defaults to `Sum`.
#[serde(default)]
pub measure_agg: HashMap<String, AggFunc>,
/// Cached formula evaluation results. Recomputed by `recompute_formulas`.
#[serde(skip)]
formula_cache: HashMap<CellKey, CellValue>,
}
/// Every field, with the two `HashMap`s in key order: a recompute refills the
/// cache in hash order, which is not a change (improvise-pc3b, gotcha 18).
impl std::fmt::Debug for Model {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let mut aggs: Vec<_> = self.measure_agg.iter().collect();
aggs.sort_by(|a, b| a.0.cmp(b.0));
let mut cache: Vec<_> = self.formula_cache.iter().collect();
cache.sort_by(|a, b| a.0.0.cmp(&b.0.0));
f.debug_struct("Model")
.field("name", &self.name)
.field("categories", &self.categories)
.field("data", &self.data)
.field("formulas", &self.formulas)
.field("next_category_id", &self.next_category_id)
.field("measure_agg", &aggs)
.field("formula_cache", &cache)
.finish()
}
}
impl Model {
pub fn new(name: impl Into<String>) -> Self {
let name = name.into();
@@ -1747,6 +1764,43 @@ mod model_tests {
);
}
/// improvise-d7aj: the memo is per KEY, not per formula — one formula met at two
/// fold keys in one evaluation. Evidence: context/test-evidence/core.md.
#[test]
fn one_formula_at_two_fold_keys_is_evaluated_at_each() {
let mut m = quarter_model(&[
("Jan", "A", 3.0),
("Feb", "A", 4.0),
("Jan", "B", 100.0),
("Feb", "B", 200.0),
]);
for text in ["G = Revenue * 2", "F = IF(Revenue > 50, G, G + B)"] {
m.add_formula(crate::formula::parse_formula(text, "_Measure").unwrap());
}
let key = coord(&[("Month", "Q1"), ("Store", "A"), ("_Measure", "F")]);
assert_eq!(
m.evaluate_covering(&key, Aggregation::Exact, &|_, _| true),
Some(CellValue::Number(614.0)),
"G at A (14) plus F at B, which is G at B (600)"
);
}
/// improvise-d7aj: a formula reached twice by two paths is finished, not in
/// progress, the second time. Evidence: context/test-evidence/core.md.
#[test]
fn a_diamond_at_a_fold_is_not_circular() {
let mut m = quarter_model(&[("Jan", "A", 3.0), ("Feb", "A", 4.0)]);
for text in ["A1 = B2 + C3", "B2 = D * 2", "C3 = D * 3", "D = Revenue"] {
m.add_formula(crate::formula::parse_formula(text, "_Measure").unwrap());
}
let key = coord(&[("Month", "Q1"), ("Store", "A"), ("_Measure", "A1")]);
super::FOLD_FORMULA_EVALUATIONS.with(|n| n.set(0));
let value = m.evaluate_covering(&key, Aggregation::Exact, &|_, _| true);
let evaluations = super::FOLD_FORMULA_EVALUATIONS.with(|n| n.get());
assert_eq!(value, Some(CellValue::Number(35.0)), "14 + 21 over D = 7");
assert_eq!(evaluations, 4, "A1, B2, C3 and D once each");
}
/// Evidence: context/test-evidence/core.md.
#[test]
fn a_groups_stored_values_fold_once_not_per_member() {
crates/improvise-core/src/view/types.rs
+1 -7
View File
@@ -1,14 +1,9 @@
use indexmap::IndexMap;
use serde::{Deserialize, Serialize};
use std::collections::{HashMap, HashSet};
use super::axis::Axis;
use crate::model::category::CategoryKind;
fn default_prune() -> bool {
true
}
/// Scratch views the app rebuilds on demand rather than persisting, because
/// they put the never-persisted `_Index`/`_Dim` on axes (improvise-60z).
pub const VIRTUAL_VIEW_NAMES: [&str; 2] = ["_Records", "_Drill"];
@@ -32,7 +27,7 @@ pub fn unused_view_name<'a>(existing: impl IntoIterator<Item = &'a str>) -> Stri
.expect("a name is free below usize::MAX, there being fewer views")
}
#[derive(Debug, Clone, Serialize, Deserialize)]
#[derive(Debug, Clone)]
pub struct View {
pub name: String,
/// Axis assignment for each category
@@ -46,7 +41,6 @@ pub struct View {
/// Number format string (e.g. ",.0f" for comma-separated integer)
pub number_format: String,
/// When true, empty rows/columns are pruned from the display.
#[serde(default = "default_prune")]
pub prune_empty: bool,
}
crates/improvise-core/src/workbook.rs
+1 -2
View File
@@ -3,13 +3,12 @@
use anyhow::{Result, anyhow};
use indexmap::IndexMap;
use serde::{Deserialize, Serialize};
use crate::model::Model;
use crate::model::category::{CategoryId, CategoryKind, ItemId};
use crate::view::{Axis, DEFAULT_VIEW, View};
#[derive(Debug, Clone, Serialize, Deserialize)]
#[derive(Debug, Clone)]
pub struct Workbook {
pub model: Model,
pub views: IndexMap<String, View>,
crates/improvise-formula/Cargo.toml
-1
View File
@@ -10,7 +10,6 @@ repository = "https://github.com/fiddlerwoaroof/improvise"
anyhow = "1"
pest = "2.9.2"
pest_derive = "2.9.2"
serde = { version = "1", features = ["derive"] }
[dev-dependencies]
pest_meta = "2.9.2"
crates/improvise-formula/src/ast.rs
+5 -8
View File
@@ -1,6 +1,4 @@
use serde::{Deserialize, Serialize};
#[derive(Debug, Clone, Serialize, Deserialize)]
#[derive(Debug, Clone)]
pub enum AggFunc {
Sum,
Avg,
@@ -11,7 +9,7 @@ pub enum AggFunc {
/// Arithmetic and comparison operators. An enum rather than a `String`, so an
/// invalid operator is caught at parse time — design-principles §2.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum BinOp {
Add,
Sub,
@@ -26,13 +24,13 @@ pub enum BinOp {
Ge,
}
#[derive(Debug, Clone, Serialize, Deserialize)]
#[derive(Debug, Clone)]
pub struct Filter {
pub category: String,
pub item: String,
}
#[derive(Debug, Clone, Serialize, Deserialize)]
#[derive(Debug, Clone)]
pub enum Expr {
Number(f64),
Ref(String),
@@ -45,7 +43,7 @@ pub enum Expr {
Is(Filter),
}
#[derive(Debug, Clone, Serialize, Deserialize)]
#[derive(Debug, Clone)]
pub struct Formula {
/// The raw formula text, e.g. "Profit = Revenue - Cost"
pub raw: String,
@@ -59,7 +57,6 @@ pub struct Formula {
pub filter: Option<Filter>,
/// How this formula's target is drawn, overriding the view's `format:` for
/// that one measure. `None` means the view answers — `context/code/core.md`.
#[serde(default)]
pub format: Option<String>,
}
crates/improvise-io/Cargo.toml
+1 -1
View File
@@ -13,7 +13,7 @@ anyhow = "1"
chrono = { version = "0.4", features = ["serde"] }
csv = "1"
flate2 = "1"
indexmap = { version = "2", features = ["serde"] }
indexmap = "2"
pest = "2.9.2"
pest_derive = "2.9.2"
serde = { version = "1", features = ["derive"] }
crates/improvise-io/src/persistence/mod.rs
+35 -33
View File
@@ -1,4 +1,4 @@
//! `.improv` document lifecycle — save, load, gzip, legacy JSON — over a
//! `.improv` document lifecycle — save, load, gzip — over a
//! [`Storage`] backend. The text format is in `format`, `parse` and `export`.
mod export;
@@ -96,11 +96,16 @@ pub fn load_from(storage: &dyn Storage, path: &Path) -> Result<Workbook> {
};
let text =
String::from_utf8(bytes).with_context(|| format!("{} is not UTF-8", path.display()))?;
// `.improv` text is the only native format; a JSON workbook is named, not
// handed to the grammar to fail on its first byte (improvise-tt22).
if text.trim_start().starts_with('{') {
serde_json::from_str(&text).context("Failed to deserialize workbook")
} else {
parse_md(&text)
anyhow::bail!(
"{} is a legacy JSON workbook, which is no longer supported: \
only .improv text can be opened",
path.display()
);
}
parse_md(&text)
}
pub fn autosave_path(path: &Path) -> std::path::PathBuf {
@@ -194,37 +199,34 @@ mod tests {
);
}
/// improvise-tt22: `.improv` text is the only native format, so a legacy
/// JSON workbook is refused by name. Evidence: `context/test-evidence/formula-and-io.md`.
#[test]
fn load_dispatcher_detects_legacy_json_by_brace() {
// The load() function routes to JSON deserializer when text starts with '{'
let m = two_cat_model();
let json = serde_json::to_string_pretty(&m).unwrap();
assert!(json.trim_start().starts_with('{'), "sanity check");
// Deserialise via the JSON path
let m2: Workbook = serde_json::from_str(&json).unwrap();
assert_eq!(m2.model.name, "Budget");
}
#[test]
fn a_legacy_json_workbook_loads_its_numbers_bit_for_bit() {
fn a_legacy_json_workbook_is_refused_by_name() {
use crate::storage::{MemoryFs, Storage};
const LOSSY: f64 = 0.248_157_142_857_142_85;
let key = coord(&[("Type", "Food"), ("Month", "Jan")]);
let mut wb = two_cat_model();
wb.model.set_cell(key.clone(), CellValue::Number(LOSSY));
let json = serde_json::to_string(&wb).unwrap();
assert!(
json.contains("0.24815714285714285"),
"the file holds the exact text"
);
let fs = MemoryFs::new();
let path = std::path::Path::new("legacy.improv");
fs.write(path, json.as_bytes()).unwrap();
let loaded = super::load_from(&fs, path).unwrap();
let Some(CellValue::Number(n)) = loaded.model.get_cell(&key) else {
panic!("the cell must load as a number");
};
assert_eq!(n.to_bits(), LOSSY.to_bits(), "loaded {n:?}, not {LOSSY:?}");
// The shape the removed loader read: `Workbook`'s serde form.
let legacy = r#"
{"model": {"name": "Budget", "categories": {}, "data": [], "formulas": []},
"views": {}, "active_view": "Default"}"#;
for name in ["legacy.improv", "legacy.improv.gz"] {
let fs = MemoryFs::new();
let path = std::path::Path::new(name);
let bytes = if name.ends_with(".gz") {
use std::io::Write;
let mut enc =
flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::default());
enc.write_all(legacy.as_bytes()).unwrap();
enc.finish().unwrap()
} else {
legacy.as_bytes().to_vec()
};
fs.write(path, &bytes).unwrap();
let err = format!("{:#}", super::load_from(&fs, path).unwrap_err());
assert!(
err.contains("legacy JSON") && err.contains("no longer supported"),
"{name}: expected the format named and refused, got {err:?}"
);
}
}
// ── save/load roundtrip via file ────────────────────────────────────
harness/opfs-contract/Cargo.lock
Generated
-3
View File
@@ -196,7 +196,6 @@ dependencies = [
"anyhow",
"pest",
"pest_derive",
"serde",
]
[[package]]
@@ -228,8 +227,6 @@ checksum = "cc4e190f5d26ca7051642629da2c52fc03bde85a03197c99408dcd291734c855"
dependencies = [
"equivalent",
"hashbrown",
"serde",
"serde_core",
]
[[package]]
tests/shipped_documents_load.rs
+36 -16
View File
@@ -3,11 +3,21 @@
use std::path::{Path, PathBuf};
fn shipped_documents() -> Vec<PathBuf> {
let root = Path::new(env!("CARGO_MANIFEST_DIR"));
let mut found: Vec<PathBuf> = ["examples", "web"]
.iter()
.flat_map(|dir| std::fs::read_dir(root.join(dir)).expect("a shipped-document directory"))
/// The directories `.gitignore` un-ignores `*.improv` in: the definition of
/// "shipped", read from the one place that states it (improvise-qlrh).
fn shipped_directories(root: &Path) -> Vec<PathBuf> {
let gitignore = std::fs::read_to_string(root.join(".gitignore")).expect("a .gitignore");
gitignore
.lines()
.filter_map(|line| line.trim().strip_prefix('!'))
.filter_map(|pattern| pattern.strip_suffix("/*.improv"))
.map(|dir| root.join(dir))
.collect()
}
fn documents_in(dir: &Path) -> Vec<PathBuf> {
let mut found: Vec<PathBuf> = std::fs::read_dir(dir)
.expect("a shipped-document directory")
.map(|entry| entry.expect("a readable directory entry").path())
// `extension()` of `x.improv.gz` is `gz`, so matching on the extension
// alone would skip it and put back the hole this test closes.
@@ -22,21 +32,31 @@ fn shipped_documents() -> Vec<PathBuf> {
#[test]
fn every_shipped_document_loads() {
let documents = shipped_documents();
let root = Path::new(env!("CARGO_MANIFEST_DIR"));
let dirs = shipped_directories(root);
assert!(
documents.len() >= 2,
"precondition: the walk must find the documents that exist, got {documents:?}"
!dirs.is_empty(),
"precondition: .gitignore must un-ignore at least one *.improv directory"
);
for path in documents {
let workbook = improvise::persistence::load(&path)
.unwrap_or_else(|e| panic!("{} does not load: {e}", path.display()));
for dir in dirs {
let documents = documents_in(&dir);
// Per directory, so a walk that silently loses one cannot pass (qlrh).
assert!(
workbook.views.contains_key(&workbook.active_view),
"{} loads but its initial view {:?} is not one of {:?}",
path.display(),
workbook.active_view,
workbook.views.keys().collect::<Vec<_>>()
!documents.is_empty(),
"precondition: {} is un-ignored for *.improv but the walk found none there",
dir.display()
);
for path in documents {
let workbook = improvise::persistence::load(&path)
.unwrap_or_else(|e| panic!("{} does not load: {e}", path.display()));
assert!(
workbook.views.contains_key(&workbook.active_view),
"{} loads but its initial view {:?} is not one of {:?}",
path.display(),
workbook.active_view,
workbook.views.keys().collect::<Vec<_>>()
);
}
}
}