Merge remote-tracking branch 'gh/main' into fix/ugj4-jibp-mouse-editor
This commit is contained in:
Edward Langley
@@ -280,8 +280,10 @@ jobs:
|
||||
# was blind to inline comments for as long as it existed (improvise-xgra).
|
||||
# fmt-workspaces is here because its own failure mode is the bug it
|
||||
# exists for: a usage error wearing a formatting failure's exit code.
|
||||
# bd-audit-ids is the dead-ID audit, whose recipe once read a bd that
|
||||
# never ran as a clean audit (improvise-52eq).
|
||||
- name: The remaining checkers refuse what they claim to
|
||||
run: ./scripts/checkers.sh pr-watch release-preflight pr-reviews fmt-workspaces
|
||||
run: ./scripts/checkers.sh pr-watch release-preflight pr-reviews fmt-workspaces bd-audit-ids
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Lints the one crate the `test` job cannot reach.
|
||||
|
||||
@@ -293,18 +293,19 @@ session before an agent tried one and found nothing. Read the ID back after
|
||||
filing, and before pushing docs run the audit:
|
||||
|
||||
```bash
|
||||
git ls-files -z --cached --others --exclude-standard -- \
|
||||
context README.md AGENTS.md CLAUDE.md crates src tests examples web harness docs \
|
||||
.agents .claude \
|
||||
| xargs -0 grep -d skip -ohE 'improvise-[a-z0-9]{3,8}\b' \
|
||||
| sort -u \
|
||||
| grep -vE '^improvise-(core|formula|io|protocol|command|gp|ws|worker|wasm|harness|registry|report|side|web)$' \
|
||||
> /tmp/bd-audit-ids.txt
|
||||
bd show $(cat /tmp/bd-audit-ids.txt) 2>&1 \
|
||||
| grep -E '^(Issue [^ ]+ not found|[Nn]o such issue)' || echo "all resolve"
|
||||
./scripts/bd-audit-ids
|
||||
```
|
||||
|
||||
`git ls-files` picks the files, and grep only reads them. The file list is the
|
||||
It exits 0, printing `all N cited ids resolve`, only when every cited id is in
|
||||
what `bd list --all` returns. A dead id is exit 1, and each one is named. A bd
|
||||
that is missing, fails, or lists no issues is exit 3. The comparison is
|
||||
positive because the recipe the script replaced was not: it searched `bd show`
|
||||
for "not found" lines and printed "all resolve" when there were none, so a bd
|
||||
that never ran read as a clean audit (improvise-52eq). Its suite,
|
||||
`scripts/bd-audit-ids-test.sh`, runs in `checkers.sh` and checks both halves:
|
||||
a dead id, and a bd that does not answer.
|
||||
|
||||
In the script, `git ls-files` picks the files, and grep only reads them. The file list is the
|
||||
tracked files plus the untracked ones, minus anything git ignores. That leaves
|
||||
out two things a `grep -r` over the same roots reads:
|
||||
|
||||
@@ -328,9 +329,6 @@ follows those symlinks and reads the skills a second time (3107 matches).
|
||||
Plain `git grep` is not a substitute: on macOS its `-E` silently ignores `\b`
|
||||
and finds a fraction of the ids.
|
||||
|
||||
The last line matches at the start of a line because `bd show` prints each
|
||||
ticket's body indented, and a body can itself contain the words "not found".
|
||||
|
||||
Three things the earlier version of this recipe got wrong, each of which let a
|
||||
real defect through:
|
||||
|
||||
@@ -344,7 +342,7 @@ real defect through:
|
||||
to match. An ID-shaped non-ID is worse than a dead ID, because the
|
||||
diagnosis is identical and the repair is not.
|
||||
- **The exclusion list named only `core` and `wasm`**, so every other crate
|
||||
name would have been reported missing. The exclusions above also cover the
|
||||
name would have been reported missing. The script's exclusions also cover the
|
||||
non-ticket tokens this repo really contains: a package name, a temp file
|
||||
name, the prose word "improvise-side", and the example crate name in the
|
||||
`kb` skill's `crate(...)` fact. **They are anchored `^…$` for the
|
||||
@@ -357,7 +355,7 @@ real defect through:
|
||||
- **A release artifact's name is ID-shaped, and `improvise-installer.sh`
|
||||
escaped only by being ten characters long.** `improvise-web.tar.gz`
|
||||
(improvise-oujb) is three, so it matches `{3,8}` and resolves to nothing;
|
||||
`web` is therefore excluded above. The general point is that the length
|
||||
`web` is therefore excluded in the script. The general point is that the length
|
||||
bound, not any rule, is what has been keeping `dist`'s artifact names out of
|
||||
this audit — so **a new artifact whose name is a short lowercase word must be
|
||||
added to the exclusion list in the same commit**, or it arrives as a dead id
|
||||
@@ -368,11 +366,12 @@ lists example ids makes itself a source of audit noise. The ids it does cite,
|
||||
improvise-8haj and improvise-oujb, are the tickets behind two of the rules
|
||||
above. They resolve, and the audit checks them like any other.
|
||||
|
||||
One environment note: in a worktree-isolated agent the shell guard refuses a
|
||||
loop that passes a runtime-computed argument to `bd`, so the recipe above
|
||||
materialises the id list into a file and hands `bd show` the whole list in one
|
||||
call rather than looping. If `bd show` in your build does not take several ids,
|
||||
run the ids through `xargs -n1 -I{} bd show {}` from a plain shell instead.
|
||||
One environment note: run the audit by its name. Which `bd` command forms a
|
||||
worktree-isolated agent's shell guard allows has varied between sessions. In
|
||||
one session it refused the old recipe's `bd show $(cat …)`, and also
|
||||
`xargs bd show < file` and `sh file`. In another, both `xargs` forms ran.
|
||||
`./scripts/bd-audit-ids` passes `bd` no argument computed at runtime, and it
|
||||
ran in the session that refused the other three.
|
||||
|
||||
A dead ID is worse than no ID: it reads as a promise that the follow-up exists.
|
||||
|
||||
|
||||
Generated
-3
@@ -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]]
|
||||
|
||||
+1
-1
@@ -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"
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -3515,3 +3515,53 @@ fixed default, not a clock read. The list is checked in both directions. An
|
||||
unlisted command that diverges fails the test, and so does a listed one that
|
||||
does not diverge. A stale entry therefore cannot linger, and a new registration
|
||||
is covered the day it lands.
|
||||
|
||||
## 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.
|
||||
|
||||
+69
-13
@@ -869,11 +869,14 @@ an involution without anything being taught about involution, and neither
|
||||
|
||||
### The cell
|
||||
|
||||
`Model::evaluate_aggregated` expands, at its head, any coordinate that names a
|
||||
group rather than an item into one key per member (a cross product where several
|
||||
coordinates do). Doing it in the **model** is what makes `display_text`, prune,
|
||||
column totals, `grid_of`, search, yank and CSV export all follow with no change.
|
||||
Three decisions:
|
||||
`Model::evaluate_covering` expands, at its head, any coordinate that names a group
|
||||
rather than an item into one key per covered member (a cross product where
|
||||
several coordinates do). The grid reads every pivot key through
|
||||
`GridLayout::evaluate`, which passes the members it draws. Since
|
||||
improvise-eccl/f2oo, display text, pruning, column totals, `grid_of`, search,
|
||||
yank and CSV export all go through that one evaluation, rather than each calling
|
||||
`Model::evaluate_aggregated`. That remains the view-less answer, and it covers
|
||||
every member. Five decisions:
|
||||
|
||||
- **Membership is read off each item's own `group` field, never off
|
||||
`Category::groups`.** `add_item_in_group` registers no `Group`; that list is
|
||||
@@ -882,14 +885,67 @@ Three decisions:
|
||||
- **An item with the same name as the group wins.** `items.contains_key`
|
||||
short-circuits the expansion, so an existing document with an item `Q1` reads
|
||||
exactly as before.
|
||||
- **Stored values fold once over every record the group covers; a derived value
|
||||
folds its members' results.** Folding the members' *aggregates* is right for
|
||||
`Sum` and wrong for `Avg`/`Count` under `Axis::None` — `avg(avg(2,4), 9) = 6`
|
||||
where the group's average is `5`. A formula target has no stored records to
|
||||
pool, so its members' evaluated results are folded with the measure's
|
||||
aggregation; for a ratio formula that is the usual pivot-table limitation, not
|
||||
a new one. Hidden items are included in the fold, as on `Axis::None`: hiding
|
||||
decides what is drawn, not what an aggregate covers.
|
||||
- **Stored values fold once over every record the group covers.** Folding the
|
||||
members' *aggregates* is right for `Sum` and wrong for `Avg`/`Count` under
|
||||
`Axis::None`: `avg(avg(2,4), 9) = 6` where the group's average is `5`.
|
||||
- **A formula is evaluated over the fold, never folded itself** (improvise-eccl).
|
||||
This bullet used to say a formula target "has no stored records to pool", so
|
||||
its members' results were folded with the measure's aggregation, and it called
|
||||
that "the usual pivot-table limitation". **The premise was wrong.** A formula's
|
||||
*inputs* are stored and can be pooled, and the model already evaluated a
|
||||
formula over pooled inputs whenever a category sat on `Axis::None`:
|
||||
`recompute_formulas` strips the None categories from the stems. Folding the
|
||||
members' results summed ratios instead, so `Margin = (Revenue - Cost) / Revenue`
|
||||
at 0.5 and 0.9 showed **1.4** on the Q1 row where Month-on-None showed **0.8**.
|
||||
Spreadsheet calculated fields do not have that limitation either. The formula
|
||||
is now evaluated at the group key, with each reference read through the fold
|
||||
(`Reach::Fold` in `model/types.rs`):
|
||||
- a raw measure reads the stored fold;
|
||||
- another formula target is evaluated over the fold in turn;
|
||||
- an aggregate inside the formula (`SUM`, `MAX`, …) pools every covered
|
||||
member's records. `MAX(Revenue)` had been *summing* the members' maxima.
|
||||
|
||||
The collapsed row now agrees with the None-axis answer. **Test with a ratio**:
|
||||
a sum agrees under both rules, which is how this was missed.
|
||||
- **The folded category reads as absent** (improvise-lnkb, the orchestrator's
|
||||
ruling). `Month IS Q1` is false on the Q1 fold, and a formula's own
|
||||
`WHERE Month = "Q1"` does not apply there. Both answers are what the None axis
|
||||
gives, and what an inline `SUM(… WHERE Month = "Q1")` already gave. This
|
||||
PR's first version said `IS` saw the group's name and called that "stated
|
||||
rather than hidden". That made the fold row the one place a formula could
|
||||
observe collapse state, breaking this section's own "fold = None over the
|
||||
drawn members" rule. `Model::read_coordinate` is the one reading of a
|
||||
coordinate by name at a fold.
|
||||
- A formula with its own `WHERE Month = "Jan"` shows **empty** on the Q1 fold,
|
||||
because the fold is not Jan. The None axis gives the same.
|
||||
- **A fold evaluates each formula once** (improvise-o1y0). A reference to
|
||||
another formula target is evaluated over the fold in turn, so without memory
|
||||
a doubling chain re-evaluates exponentially. The first version did exactly
|
||||
that: a chain of eight took 511 formula evaluations for one cell, and the
|
||||
reviewer measured 9.83 s on an 18k-cell grid. `evaluate_covering` now carries
|
||||
a memo for one evaluation, and a key met while still in progress is reported
|
||||
`circular`. Measured on an Apple M4 Max in release, on 100 products × 12
|
||||
months folded to quarters with the F0…F8 chain (4,000 cells): 0.94–1.04 s
|
||||
without the memo, about 60 ms with it, and about 10 ms on gh/main. gh/main
|
||||
was cheap because it read the members out of the formula cache, which is
|
||||
what gave the wrong ratios. The remaining ~6× is each column's cell
|
||||
re-deriving the chain below it (45 evaluations per row where 9 would do). A
|
||||
memo scoped to the whole grid would remove it; it is not built.
|
||||
- **A fold covers the members the grid draws** (improvise-f2oo). Hidden members
|
||||
are not in it. So a fold is exactly the rows it replaces, the total of a
|
||||
**stored** measure under Sum or Count does not move on `z`, and a group whose
|
||||
every member is hidden has no row. Under Avg, Min or Max a stored measure's
|
||||
total does move, and so does a **formula** column's (PR #203's review measured
|
||||
Margin 1.90 expanded, 1.30 collapsed). The strip sums the rows' values rather
|
||||
than folding the column the way a fold does, so it sums a Max row's maximum
|
||||
and a formula row's result. That is improvise-wm20, and it is not this rule. That last
|
||||
point was already true, and it contradicted the old rule, under which a hidden
|
||||
value appeared only while a sibling was visible. This deliberately **differs
|
||||
from `Axis::None`**, where hidden items still count, because the two replace
|
||||
different things. A fold replaces *rows the user sees*. A None axis replaces a
|
||||
*dimension*, and hiding an item on it hides nothing that was drawn. The view's
|
||||
hidden set reaches the model as the `covers` predicate `GridLayout::evaluate`
|
||||
passes, since the model knows no view.
|
||||
|
||||
### Settability and drill
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -846,7 +846,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`
|
||||
|
||||
@@ -5712,6 +5712,17 @@ cannot see the feature removed from `improvise-command` alone, because
|
||||
`improvise-io`, a normal dependency, enables it in the same build. That case is
|
||||
held by `every_crate_that_can_parse_json_enables_float_roundtrip` (`tooling.md`).
|
||||
|
||||
### the_grid_and_a_csv_export_agree_on_a_fold_row
|
||||
|
||||
The projection-level pin PR #203's review asked to keep. The review's own
|
||||
scratch test is made permanent here. Q1 = {Jan, Feb, Mar} with March hidden and
|
||||
funded (50/50), so a fold that covered hidden members would read 90 / 58 / 0.36.
|
||||
`grid_of` (what a client is sent) and `export_csv_to` (what a file gets) must
|
||||
both read `40.00, 8.00, 0.80` on the Q1 row. The two go through different
|
||||
producers (`viewport::grid_body` and `render_csv`), and both reach
|
||||
`GridLayout::evaluate`. Rival: that evaluation covering every member. This test
|
||||
fails with `["90.00", "58.00", "0.36"]`.
|
||||
|
||||
### p_onto_a_sparse_record_writes_it
|
||||
|
||||
improvise-q9eg, a regression introduced by PR #197 and found in its review. In
|
||||
@@ -5782,6 +5793,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`
|
||||
@@ -5806,6 +5880,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
|
||||
|
||||
@@ -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
|
||||
@@ -1346,8 +1340,8 @@ this fixture and the value is absent — this test, the next, and
|
||||
|
||||
Under `Axis::None` (`Store`), `Avg` and `Count` over `Jan/A=2, Jan/B=4, Feb/A=9`.
|
||||
The group's average is `5` and its count `3`; folding the members' aggregates gives
|
||||
`avg(3, 9) = 6` and `count(2, 1) = 2`. Rival: always fold members' results (the
|
||||
formula-target branch taken for stored values) — this test fails. `Sum` cannot see
|
||||
`avg(3, 9) = 6` and `count(2, 1) = 2`. Rival: always fold members' results (what the
|
||||
formula-target branch did before improvise-eccl) — this test fails. `Sum` cannot see
|
||||
the difference, which is why the fixture sets `measure_agg`.
|
||||
|
||||
### model::types::model_tests::an_item_named_like_a_group_is_read_as_the_item
|
||||
@@ -1412,3 +1406,108 @@ are exercised. It also lists the key's coordinates out of sorted order, which ch
|
||||
that the steps follow `CellKey`'s canonical order. Rival: `Axis::Page` in place of
|
||||
`Axis::Filter` for a fixed coordinate, which reopens improvise-dz8 (a drilled
|
||||
coordinate the user could page away from). This test fails.
|
||||
|
||||
### a_folds_ratio_formula_is_the_ratio_of_its_folded_inputs
|
||||
|
||||
improvise-eccl. `Margin = (Revenue - Cost) / Revenue` with Jan `10/5` and Feb `30/3`
|
||||
under `Q1`, and Apr in `Q2` with no data, so the Q1 fold is row 1. The member ratios
|
||||
are 0.5 and 0.9, so the three candidate rules give three different answers: summed
|
||||
1.4, averaged 0.7, pooled `(40 - 8) / 40` 0.8. The expected value is the **pooled**
|
||||
one, and it is also asserted equal to the same document with Month on `Axis::None`,
|
||||
which is the ticket's criterion. The None-axis document works because Apr carries
|
||||
no data, so the None axis pools exactly Q1's months. At `a164726f` the fold read
|
||||
**1.4**. The ticket's own fixture (0.5 and 0.5) would have separated sum from
|
||||
pool, but not mean from pool. The second formula set chains
|
||||
(`Profit = Revenue - Cost`, `Margin = Profit / Revenue`), so a reference to
|
||||
another formula target is read over the fold as well. A rule that folds a formula
|
||||
ref's members while pooling raw refs fails there.
|
||||
|
||||
### a_folds_aggregate_formula_pools_its_members_records
|
||||
|
||||
The guard on the other side of improvise-eccl. Once a formula at a fold is
|
||||
evaluated at the group key, an aggregate inside it (`SUM`, `MAX`) must pool every
|
||||
covered member's records, because `Month=Q1` matches no record. `MAX(Revenue)` is
|
||||
the discriminating case. It **failed before the change too**: the old rule folded
|
||||
the members' `MAX`es with the measure's `Sum`, giving 40 where the answer is 30.
|
||||
`SUM` alone agrees under the old rule and the new one. Rival: `contexts_of` stops
|
||||
pooling at a fold (`Reach::Fold { .. } => Vec::new()`). This test alone fails.
|
||||
|
||||
### collapsing_a_group_with_a_hidden_member_leaves_the_total_where_it_was
|
||||
|
||||
improvise-f2oo. `Q1 = {Jan 3, Feb 4}`, `Q2 = {Apr 100}`, Feb hidden. Expanded, the
|
||||
drawn rows total 103. At `a164726f` collapsing gave a Q1 fold of 7 and a total of
|
||||
**107**: `z` moved the total. The ruling: a fold covers the members the grid
|
||||
draws. The test asserts 103 both ways and a fold of 3. It then hides Jan too and
|
||||
asserts that there is no Q1 row and a total of 100. That half already held, and it
|
||||
is the inconsistency the ticket named: before, a hidden value appeared only while
|
||||
a sibling was visible. Rival: `GridLayout::evaluate` covering every member
|
||||
(`&|_, _| true`). This test alone fails, with 107.
|
||||
|
||||
### a_folds_aggregate_filtered_on_the_folded_category_counts_each_record_once
|
||||
|
||||
Found by the ChatGPT Codex review of PR #203, at head `14acf46d`. Pooling an
|
||||
aggregate over a fold's member contexts ran `agg_values` once per member, and an
|
||||
inline `WHERE` on the **folded** category overwrites that coordinate, as it does
|
||||
at a record. Every member therefore became the same record set.
|
||||
`SUM(Revenue WHERE Month = "Jan")` on the Q1 fold read **20**, Jan twice. The
|
||||
test asserts 10, and equality with the Month-on-None document. The fix dedupes
|
||||
the member contexts after the filter is applied, so each record set is pooled
|
||||
once. The rival is the pre-fix code, which read 20. `a_folds_aggregate_formula_pools_its_members_records`
|
||||
could not see this, because none of its aggregates carried a filter.
|
||||
|
||||
### a_fold_evaluates_each_formula_it_reaches_once
|
||||
|
||||
improvise-o1y0, from PR #203's review. A reference from a fold's formula to
|
||||
another formula target is evaluated over the fold in turn, with nothing
|
||||
remembered, so a doubling chain `F{i} = F{i-1} + F{i-1}` re-evaluated
|
||||
exponentially. The test **counts** formula evaluations rather than timing them,
|
||||
through a test-only thread-local counter. At `2a4f35d7` evaluating `F8` on one
|
||||
fold took **511** evaluations (2⁹ − 1); with the memo it takes 9. It also
|
||||
asserts the chain's value (7 × 256), so a memo that returned the wrong entry
|
||||
cannot pass. Rival: keep the in-progress marker but drop finished values from
|
||||
the memo (`.filter(Option::is_none)`). This test fails with 511.
|
||||
|
||||
### a_cycle_at_a_fold_is_still_circular
|
||||
|
||||
improvise-o1y0's other half. `P = Q + 1`, `Q = P + 1` at a fold must read
|
||||
`Error("circular")`. The memo's in-progress marker is what answers it now,
|
||||
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
|
||||
as absent. `Flag = Month IS "Q1"` and `Marked = Revenue * 2 WHERE Month = "Q1"`
|
||||
are each compared with the same document with Month on `Axis::None`. At
|
||||
`689c7665` `Flag` read **1** on the fold and 0 on None. The final assertion pins
|
||||
`Flag` at `Some(0.0)`, false rather than absent, so a fix that dropped the cell
|
||||
cannot pass. Rival: `read_coordinate` ignoring the reach (`if false && …`).
|
||||
This test fails.
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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.
|
||||
@@ -810,3 +811,39 @@ byte for byte (`cmp` against the committed file). With the committed lock,
|
||||
to corrupt a tracked file and call cargo, which needs the crates.io index, from
|
||||
`checkers.sh`, whose environment is scrubbed. The by-hand run above is the
|
||||
evidence.
|
||||
|
||||
### The dead-ID audit compares positively — `scripts/bd-audit-ids-test.sh` (improvise-52eq)
|
||||
|
||||
**What broke.** The audit recipe in `AGENTS.md` ended
|
||||
`bd show $(cat ids) 2>&1 | grep -E '^(Issue … not found|…)' || echo "all resolve"`.
|
||||
It looked for the *absence* of a negative line. So a bd that was missing,
|
||||
failed, or never ran printed no such line, and the audit reported "all
|
||||
resolve". In this session one attempt at the audit printed "all resolve"
|
||||
because its output file did not exist.
|
||||
|
||||
**The fix.** `scripts/bd-audit-ids` keeps the file scan and the exclusions as
|
||||
they were. It then takes the known ids from `bd list --all --limit 0 --json`
|
||||
and `comm -23`s the cited set against them. It exits 3 if bd is missing, fails,
|
||||
prints no JSON list, or lists no issues. On the tree at `12c81dd` it reports
|
||||
460 cited ids, the same count the old pipeline produced there.
|
||||
|
||||
**The suite** runs a throwaway git repo and a fake bd that answers only `list`.
|
||||
It has eight cases: a pass, a dead id (exit 1, named), four shapes of a bd that
|
||||
does not answer (no output, `[]`, a failing exit, not on PATH), the real tree
|
||||
against a silent bd, and `--help`. The passing fixture also cites
|
||||
`improvise-core` and keeps a dead id in a git-ignored `harness/serve/` file.
|
||||
So the pass needs both the exclusions and git's file list.
|
||||
|
||||
**Rivals, each run:**
|
||||
- **The old recipe**, pasted in as the script's tail: 1 passed, 7 failed.
|
||||
Every "bd did not answer" case exited 0 with "all resolve", which is the
|
||||
bug. The dead-id case failed only because the fake bd refuses `show`. That is
|
||||
one more bd that did not answer, so it is not evidence the old recipe misses a
|
||||
dead id against a working bd, which it catches.
|
||||
- **The dead-id comparison disabled** (`: >` for `comm -23`): only "a dead id
|
||||
is exit 1, and named" failed, with exit 0.
|
||||
- **An empty exclusion list**: only the passing case failed, reporting
|
||||
`improvise-core` as dead.
|
||||
|
||||
After each rival the original was restored (`cmp` against a saved copy) and the
|
||||
suite re-run at 8 passed.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
use crate::model::Aggregation;
|
||||
use crate::model::cell::{CellKey, CellValue};
|
||||
use crate::ui::effect::{self, Effect, ViewTarget};
|
||||
use crate::view::synthetic_record_info;
|
||||
@@ -585,9 +584,7 @@ impl Cmd for YankCell {
|
||||
// A records cell is the layout's one resolution, as the browser yanks it (gotcha 1).
|
||||
let value = match synthetic_record_info(&self.key) {
|
||||
Some((row, col)) => ctx.layout.records_value(row, col),
|
||||
None => ctx
|
||||
.model
|
||||
.evaluate_aggregated(&self.key, Aggregation::of(ctx.none_cats())),
|
||||
None => ctx.layout.evaluate(ctx.model, &self.key),
|
||||
};
|
||||
vec![
|
||||
Box::new(effect::SetYanked(value)),
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
use crate::model::Aggregation;
|
||||
use crate::model::cell::CellValue;
|
||||
use crate::ui::app::AppMode;
|
||||
use crate::ui::effect::{self, Effect, Panel};
|
||||
@@ -113,10 +112,7 @@ impl Cmd for SearchNavigate {
|
||||
Some(k) => k,
|
||||
None => return false,
|
||||
};
|
||||
let s = match ctx
|
||||
.model
|
||||
.evaluate_aggregated(&key, Aggregation::of(ctx.none_cats()))
|
||||
{
|
||||
let s = match ctx.layout.evaluate(ctx.model, &key) {
|
||||
Some(CellValue::Number(n)) => format!("{n}"),
|
||||
Some(CellValue::Text(t)) => t,
|
||||
Some(CellValue::Error(e)) => format!("ERR:{e}"),
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -1401,16 +1401,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
|
||||
@@ -1450,7 +1501,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);
|
||||
|
||||
@@ -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]
|
||||
|
||||
@@ -5293,6 +5293,66 @@ mod cell_identity {
|
||||
"another record's basis was invalidated for nothing"
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-eccl/f2oo at the projection boundary: the grid a client is sent and
|
||||
/// the CSV an export writes agree on a fold row — see the evidence file.
|
||||
#[test]
|
||||
fn the_grid_and_a_csv_export_agree_on_a_fold_row() {
|
||||
let mut wb = Workbook::new("T");
|
||||
wb.add_category("Month").unwrap();
|
||||
wb.add_item_in_group("Month", "Apr", "Q2").unwrap();
|
||||
for month in ["Jan", "Feb", "Mar"] {
|
||||
wb.add_item_in_group("Month", month, "Q1").unwrap();
|
||||
}
|
||||
for (month, revenue, cost) in [("Jan", 10.0, 5.0), ("Feb", 30.0, 3.0), ("Mar", 50.0, 50.0)]
|
||||
{
|
||||
for (measure, n) in [("Revenue", revenue), ("Cost", cost)] {
|
||||
wb.add_item("_Measure", measure).unwrap();
|
||||
let key = CellKey::new(vec![
|
||||
("Month".into(), month.into()),
|
||||
("_Measure".into(), measure.into()),
|
||||
]);
|
||||
wb.model.set_cell(key, CellValue::Number(n));
|
||||
}
|
||||
}
|
||||
wb.model.add_formula(
|
||||
crate::formula::parse_formula("Margin = (Revenue - Cost) / Revenue", "_Measure")
|
||||
.unwrap(),
|
||||
);
|
||||
wb.active_view_mut().set_axis("Month", Axis::Row);
|
||||
wb.active_view_mut().set_axis("_Measure", Axis::Column);
|
||||
wb.active_view_mut().hide_item("Month", "Mar");
|
||||
wb.active_view_mut().toggle_group_collapse("Month", "Q1");
|
||||
let app = App::new(wb, None);
|
||||
|
||||
let grid = grid_of(&app, WHOLE);
|
||||
let q1 = grid.row_frame.leaf_labels().iter().position(|l| l == "Q1");
|
||||
let q1 = q1.expect("a Q1 fold row");
|
||||
let drawn: Vec<String> = (0..grid.col_frame.leaf_labels().len())
|
||||
.map(|c| grid.cell(q1, c).map(|d| d.text.clone()).unwrap_or_default())
|
||||
.collect();
|
||||
|
||||
let storage = crate::storage::MemoryFs::new();
|
||||
let path = std::path::Path::new("out.csv");
|
||||
crate::persistence::export_csv_to(&storage, &app.model_state.workbook, "Default", path)
|
||||
.expect("the export writes");
|
||||
let bytes = crate::storage::Storage::read(&storage, path).unwrap();
|
||||
let csv = String::from_utf8(bytes).unwrap();
|
||||
let exported: Vec<String> = csv
|
||||
.lines()
|
||||
.find_map(|l| l.strip_prefix("Q1,"))
|
||||
.expect("a Q1 line")
|
||||
.split(',')
|
||||
.map(String::from)
|
||||
.collect();
|
||||
|
||||
assert_eq!(
|
||||
drawn,
|
||||
["40.00", "8.00", "0.80"],
|
||||
"March is hidden, so not folded"
|
||||
);
|
||||
assert_eq!(exported, drawn, "the export and the grid disagree:\n{csv}");
|
||||
}
|
||||
}
|
||||
|
||||
/// ADR 001's basis check, both arms per tier (improvise-xwi6/fkfg). The fixtures
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -6,8 +6,8 @@ use unicode_width::UnicodeWidthStr;
|
||||
use improvise_protocol::header_level_widths;
|
||||
|
||||
use crate::format::{FormatPlan, NumberFormat};
|
||||
use crate::model::Model;
|
||||
use crate::model::cell::{CellKey, CellValue};
|
||||
use crate::model::{Aggregation, Model};
|
||||
use crate::view::{AxisEntry, GridLayout};
|
||||
|
||||
/// Minimum column width — enough for short numbers/labels + 1 char gap.
|
||||
@@ -110,9 +110,7 @@ fn cell_value(
|
||||
true => (None, layout.records_value(row, col)),
|
||||
false => {
|
||||
let key = layout.cell_key(row, col);
|
||||
let value = key
|
||||
.as_ref()
|
||||
.and_then(|k| model.evaluate_aggregated(k, Aggregation::of(&layout.none_cats)));
|
||||
let value = key.as_ref().and_then(|k| layout.evaluate(model, k));
|
||||
(key, value)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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`
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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};
|
||||
@@ -51,17 +50,50 @@ fn member_records(model: &Model, partial: &CellKey) -> Vec<CellKey> {
|
||||
seen.into_iter().collect()
|
||||
}
|
||||
|
||||
/// A formula-level aggregate over the data matching `context` minus the
|
||||
/// target. Deliberately *not* [`apply_agg`]'s emptiness rule — §2.
|
||||
/// A formula-level aggregate over the data matching each of `contexts` minus the
|
||||
/// target, pooled. Deliberately *not* [`apply_agg`]'s emptiness rule — §2.
|
||||
fn eval_agg(
|
||||
model: &Model,
|
||||
context: &CellKey,
|
||||
contexts: &[CellKey],
|
||||
target_category: &str,
|
||||
func: &AggFunc,
|
||||
inner: &Expr,
|
||||
agg_filter: Option<&Filter>,
|
||||
eval_at: &dyn Fn(&Expr, &CellKey) -> Result<f64, String>,
|
||||
) -> Result<f64, String> {
|
||||
// A filter overrides its coordinate, so members it rewrites to one item name
|
||||
// one record set: pool each distinct context once (core.md §14).
|
||||
let distinct: IndexSet<CellKey> = contexts
|
||||
.iter()
|
||||
.map(|c| match agg_filter {
|
||||
Some(f) => c.clone().with(&f.category, &f.item),
|
||||
None => c.clone(),
|
||||
})
|
||||
.collect();
|
||||
let values = distinct
|
||||
.iter()
|
||||
.map(|c| agg_values(model, c, target_category, inner, agg_filter, eval_at))
|
||||
.collect::<Result<Vec<_>, String>>()?
|
||||
.concat();
|
||||
match func {
|
||||
AggFunc::Sum => Ok(values.iter().sum()),
|
||||
AggFunc::Count => Ok(values.len() as f64),
|
||||
AggFunc::Avg | AggFunc::Min | AggFunc::Max => {
|
||||
apply_agg(&values, func).ok_or_else(|| "empty".to_string())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// The values an aggregate folds at one context: a stored ref's cells, or the
|
||||
/// expression asked of each member record — §13.
|
||||
fn agg_values(
|
||||
model: &Model,
|
||||
context: &CellKey,
|
||||
target_category: &str,
|
||||
inner: &Expr,
|
||||
agg_filter: Option<&Filter>,
|
||||
eval_at: &dyn Fn(&Expr, &CellKey) -> Result<f64, String>,
|
||||
) -> Result<Vec<f64>, String> {
|
||||
let mut partial = context.without(target_category);
|
||||
// The fast path is the fold specialised to a ref that names a STORED
|
||||
// cell; where it does not, the premise fails — §13, s08t, s2w3.
|
||||
@@ -83,15 +115,15 @@ fn eval_agg(
|
||||
if let Some(f) = agg_filter {
|
||||
partial = partial.with(&f.category, &f.item);
|
||||
}
|
||||
let values: Vec<f64> = match select_raw {
|
||||
match select_raw {
|
||||
// A bare ref asks for the values that exist; anything else asks the
|
||||
// expression of each member record and folds the answers — §13.
|
||||
true => model
|
||||
true => Ok(model
|
||||
.data
|
||||
.matching_values(&partial.0)
|
||||
.into_iter()
|
||||
.filter_map(|v| v.as_f64())
|
||||
.collect(),
|
||||
.collect()),
|
||||
false => member_records(model, &partial)
|
||||
.into_iter()
|
||||
.map(|record| {
|
||||
@@ -101,28 +133,48 @@ fn eval_agg(
|
||||
.fold(partial.clone(), |k, (cat, item)| k.with(cat, item));
|
||||
eval_at(inner, &at)
|
||||
})
|
||||
.collect::<Result<Vec<f64>, String>>()?,
|
||||
};
|
||||
|
||||
match func {
|
||||
AggFunc::Sum => Ok(values.iter().sum()),
|
||||
AggFunc::Count => Ok(values.len() as f64),
|
||||
AggFunc::Avg | AggFunc::Min | AggFunc::Max => {
|
||||
apply_agg(&values, func).ok_or_else(|| "empty".to_string())
|
||||
}
|
||||
.collect(),
|
||||
}
|
||||
}
|
||||
|
||||
// How many formulas a fold has evaluated: what improvise-o1y0's regression counts.
|
||||
#[cfg(test)]
|
||||
thread_local! {
|
||||
static FOLD_FORMULA_EVALUATIONS: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
|
||||
}
|
||||
|
||||
/// One fold evaluation's memory: each formula key is evaluated once, and a key met
|
||||
/// while still in progress (`None`) is a cycle (improvise-o1y0).
|
||||
type FoldMemo = std::cell::RefCell<HashMap<CellKey, Option<Option<CellValue>>>>;
|
||||
|
||||
/// Which members of a collapsed group its fold covers. The model knows no
|
||||
/// view, so the grid passes the members it draws (core.md §14).
|
||||
pub type Covers<'a> = &'a dyn Fn(&str, &str) -> bool;
|
||||
|
||||
/// Where a formula's references are read: at one record, or across the members
|
||||
/// a collapsed group's fold covers, as a None axis reads them (core.md §14).
|
||||
#[derive(Clone, Copy)]
|
||||
enum Reach<'a> {
|
||||
Record,
|
||||
Fold {
|
||||
agg: Aggregation,
|
||||
covers: Covers<'a>,
|
||||
memo: &'a FoldMemo,
|
||||
},
|
||||
}
|
||||
|
||||
/// The **record-level** evaluator: no cache, because a member key is not a stem
|
||||
/// the cache holds. Errors propagate immediately — §3, §13.
|
||||
/// the cache holds. Errors propagate immediately — §3, §13. `reach`: §14.
|
||||
fn eval_expr(
|
||||
expr: &Expr,
|
||||
context: &CellKey,
|
||||
model: &Model,
|
||||
target_category: &str,
|
||||
depth: u8,
|
||||
reach: Reach,
|
||||
) -> Result<f64, String> {
|
||||
use crate::formula::BinOp;
|
||||
let eval = |e: &Expr| eval_expr(e, context, model, target_category, depth, reach);
|
||||
match expr {
|
||||
Expr::Number(n) => Ok(*n),
|
||||
Expr::Ref(name) => {
|
||||
@@ -130,15 +182,21 @@ fn eval_expr(
|
||||
.find_item_category(name)
|
||||
.ok_or_else(|| format!("ref:{name}"))?;
|
||||
let new_key = context.clone().with(cat, name);
|
||||
match model.evaluate_depth(&new_key, depth) {
|
||||
let value = match reach {
|
||||
Reach::Record => model.evaluate_depth(&new_key, depth),
|
||||
Reach::Fold { agg, covers, memo } => {
|
||||
model.evaluate_fold(&new_key, agg, covers, memo)
|
||||
}
|
||||
};
|
||||
match value {
|
||||
Some(CellValue::Number(n)) => Ok(n),
|
||||
Some(CellValue::Error(e)) => Err(e),
|
||||
_ => Err(format!("ref:{name}")),
|
||||
}
|
||||
}
|
||||
Expr::BinOp(op, l, r) => {
|
||||
let lv = eval_expr(l, context, model, target_category, depth)?;
|
||||
let rv = eval_expr(r, context, model, target_category, depth)?;
|
||||
let lv = eval(l)?;
|
||||
let rv = eval(r)?;
|
||||
match op {
|
||||
BinOp::Add => Ok(lv + rv),
|
||||
BinOp::Sub => Ok(lv - rv),
|
||||
@@ -153,22 +211,26 @@ fn eval_expr(
|
||||
}
|
||||
}
|
||||
}
|
||||
Expr::UnaryMinus(e) => Ok(-eval_expr(e, context, model, target_category, depth)?),
|
||||
Expr::UnaryMinus(e) => Ok(-eval(e)?),
|
||||
// A fold's aggregate pools every member record it covers, never the
|
||||
// members' own aggregates (§14).
|
||||
Expr::Agg(func, inner, agg_filter) => eval_agg(
|
||||
model,
|
||||
context,
|
||||
&model.contexts_of(context, reach),
|
||||
target_category,
|
||||
func,
|
||||
inner.as_ref(),
|
||||
agg_filter.as_ref(),
|
||||
&|e, at| eval_expr(e, at, model, target_category, depth),
|
||||
&|e, at| eval_expr(e, at, model, target_category, depth, Reach::Record),
|
||||
),
|
||||
Expr::Is(filter) => Ok(eval_is(context, filter)),
|
||||
Expr::Is(filter) => Ok(eval_is(
|
||||
model.read_coordinate(context, &filter.category, reach),
|
||||
filter,
|
||||
)),
|
||||
Expr::If(cond, then, else_) => {
|
||||
let cv = eval_bool(cond, context, model, target_category, depth)?;
|
||||
match cv {
|
||||
true => eval_expr(then, context, model, target_category, depth),
|
||||
false => eval_expr(else_, context, model, target_category, depth),
|
||||
match eval_bool(cond, context, model, target_category, depth, reach)? {
|
||||
true => eval(then),
|
||||
false => eval(else_),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -182,12 +244,13 @@ fn eval_bool(
|
||||
model: &Model,
|
||||
target_category: &str,
|
||||
depth: u8,
|
||||
reach: Reach,
|
||||
) -> Result<bool, String> {
|
||||
use crate::formula::BinOp;
|
||||
match expr {
|
||||
Expr::BinOp(op, l, r) => {
|
||||
let lv = eval_expr(l, context, model, target_category, depth)?;
|
||||
let rv = eval_expr(r, context, model, target_category, depth)?;
|
||||
let lv = eval_expr(l, context, model, target_category, depth, reach)?;
|
||||
let rv = eval_expr(r, context, model, target_category, depth, reach)?;
|
||||
match op {
|
||||
BinOp::Eq => Ok((lv - rv).abs() < FLOAT_EQ_EPSILON),
|
||||
BinOp::Ne => Ok((lv - rv).abs() >= FLOAT_EQ_EPSILON),
|
||||
@@ -211,10 +274,8 @@ fn eval_bool(
|
||||
|
||||
/// `Cat IS Item` as a number. A cell with no `Cat` coordinate is **not** that
|
||||
/// item, matching `Formula::filter`'s own `unwrap_or(false)` — §13.
|
||||
fn eval_is(context: &CellKey, filter: &Filter) -> f64 {
|
||||
context
|
||||
.get(&filter.category)
|
||||
.is_some_and(|v| v == filter.item.as_str()) as i32 as f64
|
||||
fn eval_is(coordinate: Option<&str>, filter: &Filter) -> f64 {
|
||||
coordinate.is_some_and(|v| v == filter.item.as_str()) as i32 as f64
|
||||
}
|
||||
|
||||
/// Whether a grid hides dimensions a cell key does not name. The emptiness of
|
||||
@@ -238,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>,
|
||||
@@ -247,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();
|
||||
@@ -523,33 +602,88 @@ impl Model {
|
||||
}
|
||||
|
||||
pub fn evaluate_aggregated(&self, key: &CellKey, agg: Aggregation) -> Option<CellValue> {
|
||||
let members = self.group_expansion(key);
|
||||
if !members.is_empty() {
|
||||
return self.evaluate_group(key, &members, agg);
|
||||
self.evaluate_covering(key, agg, &|_, _| true)
|
||||
}
|
||||
|
||||
/// [`Self::evaluate_aggregated`], with a collapsed group's fold covering only
|
||||
/// the members `covers` admits — the ones a grid draws (core.md §14).
|
||||
pub fn evaluate_covering(
|
||||
&self,
|
||||
key: &CellKey,
|
||||
agg: Aggregation,
|
||||
covers: Covers,
|
||||
) -> Option<CellValue> {
|
||||
self.evaluate_fold(key, agg, covers, &FoldMemo::default())
|
||||
}
|
||||
|
||||
/// A key naming a group folds; any other key reads as before. A formula is
|
||||
/// evaluated OVER the fold, never folded itself — why: core.md §14.
|
||||
fn evaluate_fold(
|
||||
&self,
|
||||
key: &CellKey,
|
||||
agg: Aggregation,
|
||||
covers: Covers,
|
||||
memo: &FoldMemo,
|
||||
) -> Option<CellValue> {
|
||||
let members = self.group_expansion(key, covers);
|
||||
match (members.is_empty(), self.formula_for(key)) {
|
||||
(true, _) => self.evaluate_unfolded(key, agg),
|
||||
(false, Some(formula)) => {
|
||||
self.fold_formula_once(formula, key, &members, (agg, covers, memo))
|
||||
}
|
||||
(false, None) => self.fold_stored(key, &members, agg),
|
||||
}
|
||||
}
|
||||
|
||||
/// A formula at a group key, evaluated at most once per fold evaluation; a key
|
||||
/// already in progress is a cycle, so `circular` still comes out (improvise-o1y0).
|
||||
fn fold_formula_once(
|
||||
&self,
|
||||
formula: &Formula,
|
||||
key: &CellKey,
|
||||
members: &[CellKey],
|
||||
(agg, covers, memo): (Aggregation, Covers, &FoldMemo),
|
||||
) -> Option<CellValue> {
|
||||
let seen = memo.borrow().get(key).cloned();
|
||||
match seen {
|
||||
Some(Some(value)) => value,
|
||||
Some(None) => Some(CellValue::Error("circular".into())),
|
||||
None => {
|
||||
memo.borrow_mut().insert(key.clone(), None);
|
||||
#[cfg(test)]
|
||||
FOLD_FORMULA_EVALUATIONS.with(|n| n.set(n.get() + 1));
|
||||
let reach = Reach::Fold { agg, covers, memo };
|
||||
let value = self.eval_formula_folded(formula, key, members, agg, reach);
|
||||
memo.borrow_mut().insert(key.clone(), Some(value.clone()));
|
||||
value
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// A key that names no group: exact, the formula cache, or raw aggregation.
|
||||
fn evaluate_unfolded(&self, key: &CellKey, agg: Aggregation) -> Option<CellValue> {
|
||||
if agg == Aggregation::Exact {
|
||||
return self.evaluate(key);
|
||||
}
|
||||
|
||||
// Check the formula cache first.
|
||||
if let Some(cached) = self.formula_cache.get(key) {
|
||||
return Some(cached.clone());
|
||||
}
|
||||
|
||||
// Aggregate raw data across all None-axis categories.
|
||||
aggregate_cell_values(&self.data.matching_values(&key.0), self.agg_for(key))
|
||||
}
|
||||
|
||||
/// One key per member where `key` names a collapsed GROUP rather than an item —
|
||||
/// the cross product over every such coordinate. Empty when none does (§14).
|
||||
fn group_expansion(&self, key: &CellKey) -> Vec<CellKey> {
|
||||
/// One key per covered member where `key` names a collapsed GROUP rather than
|
||||
/// an item — the cross product over every such coordinate. Empty when none does.
|
||||
fn group_expansion(&self, key: &CellKey, covers: Covers) -> Vec<CellKey> {
|
||||
let mut expanded = vec![key.clone()];
|
||||
for (cat_name, name) in &key.0 {
|
||||
let members = self
|
||||
let members: Vec<&str> = self
|
||||
.category(cat_name)
|
||||
.filter(|cat| !cat.items.contains_key(name.as_str()))
|
||||
.map(|cat| cat.items_in_group(name))
|
||||
.unwrap_or_default();
|
||||
.unwrap_or_default()
|
||||
.into_iter()
|
||||
.filter(|m| covers(cat_name, m))
|
||||
.collect();
|
||||
if members.is_empty() {
|
||||
continue;
|
||||
}
|
||||
@@ -564,29 +698,85 @@ impl Model {
|
||||
}
|
||||
}
|
||||
|
||||
/// A group's cell: stored values fold ONCE over every record the group covers,
|
||||
/// a derived one folds its members' results — why the two differ: core.md §14.
|
||||
fn evaluate_group(
|
||||
/// A coordinate as a formula reads it. At a fold a group's name is no item, so
|
||||
/// the folded category reads as absent, as on `Axis::None` (improvise-lnkb).
|
||||
fn read_coordinate<'k>(
|
||||
&self,
|
||||
context: &'k CellKey,
|
||||
cat: &str,
|
||||
reach: Reach,
|
||||
) -> Option<&'k str> {
|
||||
let value = context.get(cat)?;
|
||||
match reach {
|
||||
Reach::Fold { .. } if self.names_a_group(cat, value) => None,
|
||||
Reach::Fold { .. } | Reach::Record => Some(value),
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether `name` is a group of `cat` rather than one of its items.
|
||||
fn names_a_group(&self, cat: &str, name: &str) -> bool {
|
||||
self.category(cat)
|
||||
.is_some_and(|c| !c.items.contains_key(name) && !c.items_in_group(name).is_empty())
|
||||
}
|
||||
|
||||
/// The contexts an aggregate inside a formula pools over at `context`.
|
||||
fn contexts_of(&self, context: &CellKey, reach: Reach) -> Vec<CellKey> {
|
||||
let members = match reach {
|
||||
Reach::Record => Vec::new(),
|
||||
Reach::Fold { covers, .. } => self.group_expansion(context, covers),
|
||||
};
|
||||
match members.is_empty() {
|
||||
true => vec![context.clone()],
|
||||
false => members,
|
||||
}
|
||||
}
|
||||
|
||||
/// A group's stored values fold ONCE over every record the group covers.
|
||||
fn fold_stored(
|
||||
&self,
|
||||
key: &CellKey,
|
||||
members: &[CellKey],
|
||||
agg: Aggregation,
|
||||
) -> Option<CellValue> {
|
||||
let values: Vec<CellValue> = match self.formula_for(key) {
|
||||
Some(_) => members
|
||||
.iter()
|
||||
.filter_map(|k| self.evaluate_aggregated(k, agg))
|
||||
.collect(),
|
||||
None => members
|
||||
.iter()
|
||||
.flat_map(|k| match agg {
|
||||
Aggregation::Exact => self.data.get(k).into_iter().collect(),
|
||||
Aggregation::OverHidden => self.data.matching_values(&k.0),
|
||||
})
|
||||
.cloned()
|
||||
.collect(),
|
||||
};
|
||||
aggregate_cell_values(&values.iter().collect::<Vec<_>>(), self.agg_for(key))
|
||||
let values: Vec<&CellValue> = members
|
||||
.iter()
|
||||
.flat_map(|k| match agg {
|
||||
Aggregation::Exact => self.data.get(k).into_iter().collect(),
|
||||
Aggregation::OverHidden => self.data.matching_values(&k.0),
|
||||
})
|
||||
.collect();
|
||||
aggregate_cell_values(&values, self.agg_for(key))
|
||||
}
|
||||
|
||||
/// A formula at a group key, its refs read through the fold — the value the
|
||||
/// same formula has with the group's category on `Axis::None` (§14).
|
||||
fn eval_formula_folded(
|
||||
&self,
|
||||
formula: &Formula,
|
||||
key: &CellKey,
|
||||
members: &[CellKey],
|
||||
agg: Aggregation,
|
||||
reach: Reach,
|
||||
) -> Option<CellValue> {
|
||||
let applies = formula
|
||||
.filter
|
||||
.as_ref()
|
||||
.is_none_or(|f| self.read_coordinate(key, &f.category, reach) == Some(f.item.as_str()));
|
||||
if !applies {
|
||||
return self.fold_stored(key, members, agg);
|
||||
}
|
||||
let depth = Self::MAX_EVAL_DEPTH;
|
||||
match eval_expr(
|
||||
&formula.expr,
|
||||
key,
|
||||
self,
|
||||
&formula.target_category,
|
||||
depth,
|
||||
reach,
|
||||
) {
|
||||
Ok(n) => Some(CellValue::Number(n)),
|
||||
Err(e) => Some(CellValue::Error(e)),
|
||||
}
|
||||
}
|
||||
|
||||
/// [`Self::evaluate_aggregated`] as `f64`, with `0.0` for an empty cell.
|
||||
@@ -744,16 +934,19 @@ impl Model {
|
||||
)?),
|
||||
Expr::Agg(func, inner, agg_filter) => eval_agg(
|
||||
model,
|
||||
context,
|
||||
std::slice::from_ref(context),
|
||||
target_category,
|
||||
func,
|
||||
inner.as_ref(),
|
||||
agg_filter.as_ref(),
|
||||
// The fold evaluates at a MEMBER key, which is not a stem the
|
||||
// cache holds — so the record-level evaluator, in both paths.
|
||||
&|e, at| eval_expr(e, at, model, target_category, Model::MAX_EVAL_DEPTH),
|
||||
&|e, at| {
|
||||
let depth = Model::MAX_EVAL_DEPTH;
|
||||
eval_expr(e, at, model, target_category, depth, Reach::Record)
|
||||
},
|
||||
),
|
||||
Expr::Is(filter) => Ok(eval_is(context, filter)),
|
||||
Expr::Is(filter) => Ok(eval_is(context.get(&filter.category), filter)),
|
||||
Expr::If(cond, then, else_) => {
|
||||
let cv = eval_bool_cached(cond, context, model, target_category, none_cats)?;
|
||||
if cv {
|
||||
@@ -841,6 +1034,7 @@ impl Model {
|
||||
self,
|
||||
&formula.target_category,
|
||||
depth,
|
||||
Reach::Record,
|
||||
) {
|
||||
Ok(n) => Some(CellValue::Number(n)),
|
||||
Err(e) => Some(CellValue::Error(e)),
|
||||
@@ -891,7 +1085,17 @@ mod agg_helper_tests {
|
||||
let empty = Expr::Ref("Revenue".to_string());
|
||||
let ctx = CellKey::new(vec![]);
|
||||
let never = |_: &Expr, _: &CellKey| unreachable!("a bare Ref never folds per record");
|
||||
let run = |func| eval_agg(&model, &ctx, "_Measure", &func, &empty, None, &never);
|
||||
let run = |func| {
|
||||
eval_agg(
|
||||
&model,
|
||||
std::slice::from_ref(&ctx),
|
||||
"_Measure",
|
||||
&func,
|
||||
&empty,
|
||||
None,
|
||||
&never,
|
||||
)
|
||||
};
|
||||
|
||||
assert_eq!(run(AggFunc::Sum), Ok(0.0));
|
||||
assert_eq!(run(AggFunc::Count), Ok(0.0));
|
||||
@@ -1493,6 +1697,78 @@ mod model_tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-o1y0: a fold evaluates each formula it reaches once — see
|
||||
/// context/test-evidence/core.md.
|
||||
#[test]
|
||||
fn a_fold_evaluates_each_formula_it_reaches_once() {
|
||||
let mut m = quarter_model(&[("Jan", "A", 3.0), ("Feb", "A", 4.0)]);
|
||||
m.add_formula(crate::formula::parse_formula("F0 = Revenue", "_Measure").unwrap());
|
||||
for i in 1..=8 {
|
||||
let text = format!("F{i} = F{p} + F{p}", p = i - 1);
|
||||
m.add_formula(crate::formula::parse_formula(&text, "_Measure").unwrap());
|
||||
}
|
||||
let key = coord(&[("Month", "Q1"), ("Store", "A"), ("_Measure", "F8")]);
|
||||
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(7.0 * 256.0)),
|
||||
"the chain's value"
|
||||
);
|
||||
assert_eq!(evaluations, 9, "F0..F8 once each, not 2^9 - 1");
|
||||
}
|
||||
|
||||
/// improvise-o1y0's other half: a key met while still in progress is a cycle.
|
||||
#[test]
|
||||
fn a_cycle_at_a_fold_is_still_circular() {
|
||||
let mut m = quarter_model(&[("Jan", "A", 3.0), ("Feb", "A", 4.0)]);
|
||||
m.add_formula(crate::formula::parse_formula("P = Q + 1", "_Measure").unwrap());
|
||||
m.add_formula(crate::formula::parse_formula("Q = P + 1", "_Measure").unwrap());
|
||||
let key = coord(&[("Month", "Q1"), ("Store", "A"), ("_Measure", "P")]);
|
||||
assert_eq!(
|
||||
m.evaluate_covering(&key, Aggregation::Exact, &|_, _| true),
|
||||
Some(CellValue::Error("circular".into()))
|
||||
);
|
||||
}
|
||||
|
||||
/// 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() {
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use std::collections::{HashMap, HashSet};
|
||||
use std::rc::Rc;
|
||||
|
||||
use crate::model::category::{DIM_CATEGORY, INDEX_CATEGORY, MEASURE_CATEGORY};
|
||||
@@ -377,6 +378,8 @@ pub struct GridLayout {
|
||||
/// In records mode: what each column holds, parallel to `col_items`.
|
||||
/// Empty for normal pivot views.
|
||||
record_cols: Vec<RecordCol>,
|
||||
/// The view's hidden items: a fold covers only the members this grid draws.
|
||||
hidden: HashMap<String, HashSet<String>>,
|
||||
}
|
||||
|
||||
impl GridLayout {
|
||||
@@ -460,6 +463,7 @@ impl GridLayout {
|
||||
none_cats,
|
||||
records: None,
|
||||
record_cols: Vec::new(),
|
||||
hidden: view.hidden_items.clone(),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -493,6 +497,7 @@ impl GridLayout {
|
||||
none_cats,
|
||||
records: Some(Rc::new(records)),
|
||||
record_cols,
|
||||
hidden: HashMap::new(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -611,7 +616,7 @@ impl GridLayout {
|
||||
for (ci, cell) in row.iter_mut().enumerate() {
|
||||
*cell = self
|
||||
.cell_key(ri, ci)
|
||||
.and_then(|k| model.evaluate_aggregated(&k, Aggregation::of(&self.none_cats)))
|
||||
.and_then(|k| self.evaluate(model, &k))
|
||||
.is_some();
|
||||
}
|
||||
}
|
||||
@@ -649,6 +654,13 @@ impl GridLayout {
|
||||
axis_label(&self.col_items, col)
|
||||
}
|
||||
|
||||
/// The value of a pivot key as THIS grid reads it: aggregated over its None
|
||||
/// axis, and a fold over the members it draws (core.md §14). One evaluation.
|
||||
pub fn evaluate(&self, model: &Model, key: &CellKey) -> Option<CellValue> {
|
||||
let drawn = |cat: &str, item: &str| !self.hidden.get(cat).is_some_and(|h| h.contains(item));
|
||||
model.evaluate_covering(key, Aggregation::of(&self.none_cats), &drawn)
|
||||
}
|
||||
|
||||
/// Display text for (row, col) in either mode.
|
||||
pub fn display_text(
|
||||
&self,
|
||||
@@ -662,8 +674,7 @@ impl GridLayout {
|
||||
} else {
|
||||
self.cell_key(row, col)
|
||||
.and_then(|key| {
|
||||
let value =
|
||||
model.evaluate_aggregated(&key, Aggregation::of(&self.none_cats))?;
|
||||
let value = self.evaluate(model, &key)?;
|
||||
Some(plan.at(Some(&key)).value(Some(&value)))
|
||||
})
|
||||
.unwrap_or_default()
|
||||
@@ -1780,6 +1791,181 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// Q2 = {Apr}, Q1 = {Jan, Feb}, so the Q1 fold is row 1. Jan and Feb carry
|
||||
/// ratios 0.5 and 0.9 whose sum, mean and pooled ratio all differ; Apr is empty.
|
||||
fn margin_book(formulas: &[&str]) -> Workbook {
|
||||
let mut wb = Workbook::new("T");
|
||||
wb.add_category("Month").unwrap();
|
||||
wb.add_item_in_group("Month", "Apr", "Q2").unwrap();
|
||||
wb.add_item_in_group("Month", "Jan", "Q1").unwrap();
|
||||
wb.add_item_in_group("Month", "Feb", "Q1").unwrap();
|
||||
wb.add_item("_Measure", "Revenue").unwrap();
|
||||
wb.add_item("_Measure", "Cost").unwrap();
|
||||
for (month, revenue, cost) in [("Jan", 10.0, 5.0), ("Feb", 30.0, 3.0)] {
|
||||
for (measure, n) in [("Revenue", revenue), ("Cost", cost)] {
|
||||
wb.model.set_cell(
|
||||
CellKey::new(vec![
|
||||
("Month".into(), month.into()),
|
||||
("_Measure".into(), measure.into()),
|
||||
]),
|
||||
CellValue::Number(n),
|
||||
);
|
||||
}
|
||||
}
|
||||
for f in formulas {
|
||||
wb.model
|
||||
.add_formula(crate::formula::parse_formula(f, "_Measure").unwrap());
|
||||
}
|
||||
wb.active_view_mut().set_axis("Month", Axis::Row);
|
||||
wb.active_view_mut().set_axis("_Measure", Axis::Column);
|
||||
wb
|
||||
}
|
||||
|
||||
/// `measure` at the row labelled `row_label`, as the grid reads it.
|
||||
fn grid_value(wb: &mut Workbook, row_label: &str, measure: &str) -> Option<f64> {
|
||||
wb.model.recompute_formulas(&wb.active_view().none_cats());
|
||||
let layout = GridLayout::new(&wb.model, wb.active_view());
|
||||
let row = (0..layout.row_count()).find(|&r| layout.row_label(r) == row_label)?;
|
||||
let col = (0..layout.col_count()).find(|&c| layout.col_label(c) == measure)?;
|
||||
layout
|
||||
.evaluate(&wb.model, &layout.cell_key(row, col)?)
|
||||
.and_then(|v| v.as_f64())
|
||||
}
|
||||
|
||||
/// improvise-eccl — see context/test-evidence/core.md.
|
||||
#[test]
|
||||
fn a_folds_ratio_formula_is_the_ratio_of_its_folded_inputs() {
|
||||
for formulas in [
|
||||
&["Margin = (Revenue - Cost) / Revenue"][..],
|
||||
&["Profit = Revenue - Cost", "Margin = Profit / Revenue"][..],
|
||||
] {
|
||||
let mut folded = margin_book(formulas);
|
||||
folded
|
||||
.active_view_mut()
|
||||
.toggle_group_collapse("Month", "Q1");
|
||||
let fold = grid_value(&mut folded, "Q1", "Margin").expect("a Q1 fold");
|
||||
|
||||
let mut none = margin_book(formulas);
|
||||
none.active_view_mut().set_axis("Month", Axis::None);
|
||||
let over_none = grid_value(&mut none, "", "Margin").expect("a None-axis cell");
|
||||
|
||||
assert!((fold - 0.8).abs() < 1e-9, "{formulas:?}: fold {fold}");
|
||||
assert!(
|
||||
(fold - over_none).abs() < 1e-9,
|
||||
"{formulas:?}: {fold} vs {over_none}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// improvise-lnkb: the folded category reads as absent, as on `Axis::None` — see
|
||||
/// context/test-evidence/core.md.
|
||||
#[test]
|
||||
fn a_fold_answers_is_and_where_on_its_category_as_the_none_axis_does() {
|
||||
let formulas = [
|
||||
"Flag = Month IS \"Q1\"",
|
||||
"Marked = Revenue * 2 WHERE Month = \"Q1\"",
|
||||
];
|
||||
for measure in ["Flag", "Marked"] {
|
||||
let mut folded = margin_book(&formulas);
|
||||
folded
|
||||
.active_view_mut()
|
||||
.toggle_group_collapse("Month", "Q1");
|
||||
let fold = grid_value(&mut folded, "Q1", measure);
|
||||
let mut none = margin_book(&formulas);
|
||||
none.active_view_mut().set_axis("Month", Axis::None);
|
||||
let over_none = grid_value(&mut none, "", measure);
|
||||
assert_eq!(
|
||||
fold, over_none,
|
||||
"{measure}: the fold and the None axis differ"
|
||||
);
|
||||
}
|
||||
let mut folded = margin_book(&formulas);
|
||||
folded
|
||||
.active_view_mut()
|
||||
.toggle_group_collapse("Month", "Q1");
|
||||
assert_eq!(
|
||||
grid_value(&mut folded, "Q1", "Flag"),
|
||||
Some(0.0),
|
||||
"false, not absent"
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-eccl's guard on the other side: an aggregate INSIDE the formula
|
||||
/// still pools every record the fold covers.
|
||||
#[test]
|
||||
fn a_folds_aggregate_formula_pools_its_members_records() {
|
||||
let mut wb = margin_book(&["Top = MAX(Revenue)", "Total = SUM(Revenue)"]);
|
||||
wb.active_view_mut().toggle_group_collapse("Month", "Q1");
|
||||
assert_eq!(grid_value(&mut wb, "Q1", "Total"), Some(40.0));
|
||||
assert_eq!(grid_value(&mut wb, "Q1", "Top"), Some(30.0));
|
||||
}
|
||||
|
||||
/// An inline `WHERE` on the folded category makes every member context the
|
||||
/// same record set — see context/test-evidence/core.md.
|
||||
#[test]
|
||||
fn a_folds_aggregate_filtered_on_the_folded_category_counts_each_record_once() {
|
||||
let formula = ["Jans = SUM(Revenue WHERE Month = \"Jan\")"];
|
||||
let mut folded = margin_book(&formula);
|
||||
folded
|
||||
.active_view_mut()
|
||||
.toggle_group_collapse("Month", "Q1");
|
||||
let fold = grid_value(&mut folded, "Q1", "Jans");
|
||||
|
||||
let mut none = margin_book(&formula);
|
||||
none.active_view_mut().set_axis("Month", Axis::None);
|
||||
assert_eq!(fold, Some(10.0), "Jan's revenue, once");
|
||||
assert_eq!(
|
||||
fold,
|
||||
grid_value(&mut none, "", "Jans"),
|
||||
"the None-axis answer"
|
||||
);
|
||||
}
|
||||
|
||||
/// Every row's value in column 0, summed: the totals strip's arithmetic.
|
||||
fn column_total(wb: &Workbook) -> f64 {
|
||||
let layout = GridLayout::new(&wb.model, wb.active_view());
|
||||
(0..layout.row_count())
|
||||
.filter_map(|r| layout.evaluate(&wb.model, &layout.cell_key(r, 0)?))
|
||||
.filter_map(|v| v.as_f64())
|
||||
.sum()
|
||||
}
|
||||
|
||||
/// improvise-f2oo — see context/test-evidence/core.md.
|
||||
#[test]
|
||||
fn collapsing_a_group_with_a_hidden_member_leaves_the_total_where_it_was() {
|
||||
let mut wb = Workbook::new("T");
|
||||
wb.add_category("Month").unwrap();
|
||||
wb.add_category("Type").unwrap();
|
||||
wb.add_item("Type", "Food").unwrap();
|
||||
for (month, quarter, n) in [("Jan", "Q1", 3.0), ("Feb", "Q1", 4.0), ("Apr", "Q2", 100.0)] {
|
||||
wb.add_item_in_group("Month", month, quarter).unwrap();
|
||||
wb.model.set_cell(
|
||||
CellKey::new(vec![
|
||||
("Month".into(), month.into()),
|
||||
("Type".into(), "Food".into()),
|
||||
]),
|
||||
CellValue::Number(n),
|
||||
);
|
||||
}
|
||||
wb.active_view_mut().set_axis("Month", Axis::Row);
|
||||
wb.active_view_mut().set_axis("Type", Axis::Column);
|
||||
wb.active_view_mut().hide_item("Month", "Feb");
|
||||
assert_eq!(column_total(&wb), 103.0, "expanded: Jan and Apr are drawn");
|
||||
|
||||
wb.active_view_mut().toggle_group_collapse("Month", "Q1");
|
||||
assert_eq!(column_total(&wb), 103.0, "z moved the total");
|
||||
assert_eq!(
|
||||
grid_value(&mut wb, "Q1", "Food"),
|
||||
Some(3.0),
|
||||
"Feb is hidden"
|
||||
);
|
||||
|
||||
// Every member hidden: no fold is drawn, and the total still agrees.
|
||||
wb.active_view_mut().hide_item("Month", "Jan");
|
||||
assert_eq!(grid_value(&mut wb, "Q1", "Food"), None, "no Q1 row");
|
||||
assert_eq!(column_total(&wb), 100.0);
|
||||
}
|
||||
|
||||
/// improvise-t0ma: the steps themselves, not their translation into effects —
|
||||
/// see context/test-evidence/core.md.
|
||||
#[test]
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
|
||||
|
||||
@@ -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>,
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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>,
|
||||
}
|
||||
|
||||
|
||||
@@ -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"] }
|
||||
|
||||
@@ -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 ────────────────────────────────────
|
||||
|
||||
Generated
-3
@@ -196,7 +196,6 @@ dependencies = [
|
||||
"anyhow",
|
||||
"pest",
|
||||
"pest_derive",
|
||||
"serde",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -228,8 +227,6 @@ checksum = "cc4e190f5d26ca7051642629da2c52fc03bde85a03197c99408dcd291734c855"
|
||||
dependencies = [
|
||||
"equivalent",
|
||||
"hashbrown",
|
||||
"serde",
|
||||
"serde_core",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
|
||||
Executable
+75
@@ -0,0 +1,75 @@
|
||||
#!/usr/bin/env bash
|
||||
# The dead-ID audit: every improvise-<id> the tree cites must be a ticket bd
|
||||
# knows. See AGENTS.md, "Cite a ticket ID you have read back" (improvise-52eq).
|
||||
set -euo pipefail
|
||||
|
||||
usage() {
|
||||
cat >&2 <<'EOF'
|
||||
usage: bd-audit-ids
|
||||
|
||||
Run from anywhere inside the repository. Takes no arguments.
|
||||
|
||||
exit 0 every cited id is a ticket bd lists
|
||||
exit 1 at least one cited id is not; each is printed
|
||||
exit 3 bd could not answer: missing, failing, or listing nothing
|
||||
exit 4 usage error, or the file scan itself failed
|
||||
|
||||
The comparison is POSITIVE: a cited id passes only by appearing in what
|
||||
`bd list --all` returns. The recipe this replaced looked for bd's "not found"
|
||||
lines and reported "all resolve" when there were none, so a bd that never ran
|
||||
read as a clean audit.
|
||||
|
||||
THIS SCRIPT'S `$?` MUST BE THE ONE YOU READ. Nothing may follow it in the same
|
||||
pipeline or `;` list.
|
||||
EOF
|
||||
exit 4
|
||||
}
|
||||
[ $# -eq 0 ] || usage
|
||||
|
||||
BD="${BD_AUDIT_BD:-bd}"
|
||||
ROOTS=(context README.md AGENTS.md CLAUDE.md crates src tests examples web
|
||||
harness docs .agents .claude)
|
||||
# Crate, package, artifact and file names that are shaped like ids. Anchored,
|
||||
# because an unanchored `gp` would silently exclude any real ticket id starting with it.
|
||||
EXCLUDE='^improvise-(core|formula|io|protocol|command|gp|ws|worker|wasm|harness|registry|report|side|web)$'
|
||||
|
||||
work=$(mktemp -d)
|
||||
trap 'rm -rf "$work"' EXIT
|
||||
|
||||
cd "$(git rev-parse --show-toplevel)"
|
||||
git ls-files -z --cached --others --exclude-standard -- "${ROOTS[@]}" > "$work/files"
|
||||
|
||||
# xargs exits 123 when one grep batch matched nothing, which is not an error.
|
||||
set +e
|
||||
xargs -0 grep -d skip -ohE 'improvise-[a-z0-9]{3,8}\b' < "$work/files" > "$work/raw"
|
||||
rc=$?
|
||||
set -e
|
||||
[ "$rc" -eq 0 ] || [ "$rc" -eq 123 ] || { echo "bd-audit-ids: grep failed ($rc)" >&2; exit 4; }
|
||||
sort -u "$work/raw" | { grep -vE "$EXCLUDE" || true; } > "$work/cited"
|
||||
|
||||
if ! command -v "$BD" >/dev/null 2>&1; then
|
||||
echo "bd-audit-ids: '$BD' is not on PATH, so nothing was checked." >&2
|
||||
exit 3
|
||||
fi
|
||||
if ! "$BD" list --all --limit 0 --json > "$work/known.json" 2> "$work/bd.err"; then
|
||||
echo "bd-audit-ids: '$BD list' failed, so nothing was checked:" >&2
|
||||
cat "$work/bd.err" >&2
|
||||
exit 3
|
||||
fi
|
||||
if ! jq -r '.[].id' "$work/known.json" 2>/dev/null | sort -u > "$work/known"; then
|
||||
echo "bd-audit-ids: '$BD list' did not print a JSON list of issues." >&2
|
||||
exit 3
|
||||
fi
|
||||
if [ ! -s "$work/known" ]; then
|
||||
echo "bd-audit-ids: '$BD list' returned no issues; refusing to call that a pass." >&2
|
||||
exit 3
|
||||
fi
|
||||
|
||||
comm -23 "$work/cited" "$work/known" > "$work/dead"
|
||||
if [ -s "$work/dead" ]; then
|
||||
echo "bd-audit-ids: cited but not a ticket bd knows:"
|
||||
sed 's/^/ /' "$work/dead"
|
||||
exit 1
|
||||
fi
|
||||
printf 'bd-audit-ids: all %d cited ids resolve (bd knows %d)\n' \
|
||||
"$(wc -l < "$work/cited" | tr -d ' ')" "$(wc -l < "$work/known" | tr -d ' ')"
|
||||
Executable
+87
@@ -0,0 +1,87 @@
|
||||
#!/usr/bin/env bash
|
||||
# check_the_checker for scripts/bd-audit-ids. A throwaway git repo and a fake
|
||||
# bd; no network, no real tracker. See context/test-evidence/tooling.md.
|
||||
set -uo pipefail
|
||||
|
||||
here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||
AUDIT="$here/bd-audit-ids"
|
||||
work="$(mktemp -d)"
|
||||
trap 'rm -rf "$work"' EXIT
|
||||
|
||||
pass=0; fail=0
|
||||
ok() { printf ' ok %s\n' "$1"; pass=$((pass+1)); }
|
||||
bad() { printf ' FAIL %s (%s)\n' "$1" "$2"; fail=$((fail+1)); }
|
||||
|
||||
# The fake bd answers `list` from $FAKE_BD_OUT and exits $FAKE_BD_RC; any
|
||||
# other subcommand is an error, so the audit cannot pass by calling `show`.
|
||||
mkdir -p "$work/bin"
|
||||
cat > "$work/bin/bd" <<'EOF'
|
||||
#!/usr/bin/env bash
|
||||
[ "${1:-}" = list ] || { echo "fake bd: unexpected '$*'" >&2; exit 2; }
|
||||
[ -f "${FAKE_BD_OUT:-}" ] && cat "$FAKE_BD_OUT"
|
||||
exit "${FAKE_BD_RC:-0}"
|
||||
EOF
|
||||
chmod +x "$work/bin/bd"
|
||||
|
||||
repo="$work/repo"
|
||||
mkdir -p "$repo/context" "$repo/harness/serve"
|
||||
git -C "$repo" init -q
|
||||
printf 'serve/\n' > "$repo/harness/.gitignore"
|
||||
# A live id, and a crate name the exclusion list must drop: without the
|
||||
# exclusion, improvise-core would be reported dead and the first case fails.
|
||||
printf 'See improvise-ab12 in improvise-core.\n' > "$repo/context/notes.md"
|
||||
# Ignored build output with a dead id. The audit reads git's file list, so
|
||||
# this must never be reported (improvise-8haj).
|
||||
printf 'improvise-gone9\n' > "$repo/harness/serve/copy.md"
|
||||
|
||||
printf '[{"id":"improvise-ab12"},{"id":"improvise-cd34"}]\n' > "$work/known.json"
|
||||
printf '' > "$work/empty"
|
||||
printf '[]\n' > "$work/none.json"
|
||||
|
||||
# $1 name, $2 expected exit, $3 grep -E pattern the output must contain;
|
||||
# the caller sets FAKE_BD_* and BD_AUDIT_BD in the environment.
|
||||
run_case() {
|
||||
local name="$1" want="$2" pat="$3"
|
||||
(cd "$repo" && "$AUDIT") > "$work/out" 2>&1
|
||||
local got=$?
|
||||
if [ "$got" != "$want" ]; then bad "$name" "exit $got, wanted $want: $(head -c 200 "$work/out")"
|
||||
elif ! grep -Eq "$pat" "$work/out"; then bad "$name" "no /$pat/ in: $(head -c 200 "$work/out")"
|
||||
else ok "$name"; fi
|
||||
}
|
||||
|
||||
export BD_AUDIT_BD="$work/bin/bd"
|
||||
|
||||
FAKE_BD_OUT="$work/known.json" FAKE_BD_RC=0 \
|
||||
run_case "every cited id known is a pass" 0 'all 1 cited ids resolve'
|
||||
|
||||
printf 'Also improvise-zzzq.\n' > "$repo/context/dead.md"
|
||||
FAKE_BD_OUT="$work/known.json" FAKE_BD_RC=0 \
|
||||
run_case "a dead id is exit 1, and named" 1 '^ improvise-zzzq$'
|
||||
rm -f "$repo/context/dead.md"
|
||||
|
||||
# The three shapes of "bd never answered". The recipe this replaced reported
|
||||
# "all resolve" for every one of them (improvise-52eq).
|
||||
FAKE_BD_OUT="$work/empty" FAKE_BD_RC=0 \
|
||||
run_case "a bd that prints nothing is not a pass" 3 'did not print a JSON list|returned no issues'
|
||||
FAKE_BD_OUT="$work/none.json" FAKE_BD_RC=0 \
|
||||
run_case "a bd that lists no issues is not a pass" 3 'returned no issues'
|
||||
FAKE_BD_OUT="$work/known.json" FAKE_BD_RC=1 \
|
||||
run_case "a bd that fails is not a pass" 3 'list. failed'
|
||||
BD_AUDIT_BD="$work/bin/no-such-bd" \
|
||||
run_case "a missing bd is not a pass" 3 'is not on PATH'
|
||||
|
||||
# The live repo, with the fake bd listing nothing: if this passed, a real
|
||||
# audit whose bd fell silent would pass too.
|
||||
(cd "$here/.." && FAKE_BD_OUT="$work/none.json" "$AUDIT") > "$work/out" 2>&1
|
||||
got=$?
|
||||
[ "$got" = 3 ] && ok "the real tree does not pass against a silent bd" \
|
||||
|| bad "the real tree does not pass against a silent bd" "exit $got"
|
||||
|
||||
"$AUDIT" --help > "$work/out" 2>&1
|
||||
got=$?
|
||||
[ "$got" = 4 ] && grep -q 'POSITIVE' "$work/out" \
|
||||
&& ok "--help is exit 4 and states the positive comparison" \
|
||||
|| bad "--help is exit 4 and states the positive comparison" "exit $got"
|
||||
|
||||
printf '\nbd-audit-ids-test: %d passed, %d failed\n' "$pass" "$fail"
|
||||
[ "$fail" -eq 0 ]
|
||||
@@ -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<_>>()
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user