Merge pull request #224 from fiddlerwoaroof/wave/tt22-no-legacy-json

.improv is the only native format: remove the legacy JSON document loader (improvise-tt22)
This commit is contained in:
Ed Langley
2026-09-23 08:41:29 -07:00
committed by GitHub
21 changed files with 231 additions and 136 deletions
Generated
-3
View File
@@ -897,7 +897,6 @@ dependencies = [
"pest_derive", "pest_derive",
"pest_meta", "pest_meta",
"proptest", "proptest",
"serde",
] ]
[[package]] [[package]]
@@ -997,8 +996,6 @@ checksum = "cc4e190f5d26ca7051642629da2c52fc03bde85a03197c99408dcd291734c855"
dependencies = [ dependencies = [
"equivalent", "equivalent",
"hashbrown 0.17.1", "hashbrown 0.17.1",
"serde",
"serde_core",
] ]
[[package]] [[package]]
+1 -1
View File
@@ -45,7 +45,7 @@ serde = { version = "1", features = ["derive"] }
serde_json = { version = "1", features = ["float_roundtrip"] } serde_json = { version = "1", features = ["float_roundtrip"] }
anyhow = "1" anyhow = "1"
thiserror = "2" thiserror = "2"
indexmap = { version = "2", features = ["serde"] } indexmap = "2"
chrono = { version = "0.4", features = ["serde"] } chrono = { version = "0.4", features = ["serde"] }
flate2 = "1" flate2 = "1"
unicode-width = "^0.2" unicode-width = "^0.2"
+3 -2
View File
@@ -169,8 +169,9 @@ Measure: column
format: ,.2f format: ,.2f
``` ```
Gzip-compressed `.improv.gz` is also supported. Legacy JSON is auto-detected Gzip-compressed `.improv.gz` is also supported. `.improv` text is the only
for backward compatibility. 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 ## Import and scripting
+40 -2
View File
@@ -471,8 +471,46 @@ document that decompressed perfectly well. Decoding once, afterwards, also
leaves **one** answer to "is this UTF-8" rather than one per branch. 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 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 document name, not of the filesystem (improvise-6mq).
a leading `{` and is never written.
**`.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 `parse_number_cell` returns `CellValue::Error` naming the offending text rather
than a fabricated `0.0`. No token the current grammar admits actually fails than a fabricated `0.0`. No token the current grammar admits actually fails
+1 -1
View File
@@ -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 **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. 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. 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 (`StartImportWizard`, `ImportJsonHeadless`) and the root binary's
`improvise import` all parsed with the lossy default, and in the TUI binary `improvise import` all parsed with the lossy default, and in the TUI binary
nothing enabled the feature (improvise-6yvg, reproduced: `0.24815714285714285` nothing enabled the feature (improvise-6yvg, reproduced: `0.24815714285714285`
+27
View File
@@ -5880,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 buffers, cursor), so a `Snapshot` would drop exactly the state those tests
stage. This test is what makes the copy faithful at startup. 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 ### 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 improvise-y5eo. `App::with_storage` set `view_state.mode` to `Normal` and never
-6
View File
@@ -476,12 +476,6 @@ cell in the row).
The pair `live_records` yields is not two facts that could drift apart: the 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. 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` ## `model/record.rs`
### a_records_surrogate_is_pinned_to_a_literal ### a_records_surrogate_is_pinned_to_a_literal
+28 -10
View File
@@ -1435,16 +1435,34 @@ reintroduce the iot6 divergence between what the user sees and what they export.
On the pre-6qet tree the body is empty (the group had no row, and pruning left On the pre-6qet tree the body is empty (the group had no row, and pruning left
nothing), so the test fails there. 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 ### `a_legacy_json_workbook_is_refused_by_name`
`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.
**Red at `1158c23`** in `cargo test -p improvise-io`: loaded improvise-tt22, on the user's direction on PR #210: `.improv` text is the only
`0.24815714285714283`. Green once `improvise-io`'s own manifest enables native format. A document starting with `{` used to be deserialized as a
`float_roundtrip`, and red again with only that line reverted. `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.
+3 -2
View File
@@ -696,8 +696,9 @@ every target-specific entry, whose **normal** serde_json dependency lacks
**Faults injected, each run:** **Faults injected, each run:**
- **The feature removed from `improvise-io` alone.** The check fails naming - **The feature removed from `improvise-io` alone.** The check fails naming
`improvise-io (all targets)`, and so does `improvise-io (all targets)`. So did
`a_legacy_json_workbook_loads_its_numbers_bit_for_bit`. `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 `a_json_import_stores_its_numbers_bit_for_bit` stays green, because
`improvise-command` enables the feature and unification reaches io in its `improvise-command` enables the feature and unification reaches io in its
build. build.
+58 -6
View File
@@ -1401,16 +1401,67 @@ mod tests {
// ── Effect classification (improvise-mae) ─────────────────────────── // ── Effect classification (improvise-mae) ───────────────────────────
/// Serialized model data — everything a connected client renders. /// Every field of the model, rendered — everything a connected client
/// Excludes the view slice, which is per-client. /// renders. `Debug`, not serde: the model has no serde form (improvise-tt22).
fn model_fingerprint(app: &App) -> String { 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). /// hash stricter than `document_config` is wanted (improvise-1xf4).
fn view_config_fingerprint(app: &App) -> String { 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 /// improvise-mae: rather than restate each declaration, apply the effect and
@@ -1450,7 +1501,8 @@ mod tests {
EffectKind::ViewOnly, EffectKind::ViewOnly,
"{effect:?} is in the ViewOnly case list" "{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 model_before = model_fingerprint(&app);
let views_before = view_config_fingerprint(&app); let views_before = view_config_fingerprint(&app);
effect.apply(&mut app); effect.apply(&mut app);
+1 -1
View File
@@ -9,7 +9,7 @@ repository = "https://github.com/fiddlerwoaroof/improvise"
[dependencies] [dependencies]
improvise-formula = { path = "../improvise-formula" } improvise-formula = { path = "../improvise-formula" }
anyhow = "1" anyhow = "1"
indexmap = { version = "2", features = ["serde"] } indexmap = "2"
serde = { version = "1", features = ["derive"] } serde = { version = "1", features = ["derive"] }
# SipHash-2-4 under a constant key: the record surrogate must not move # SipHash-2-4 under a constant key: the record surrogate must not move
# between processes, which rules out anything built on `RandomState` # between processes, which rules out anything built on `RandomState`
+4 -6
View File
@@ -1,10 +1,9 @@
use indexmap::IndexMap; use indexmap::IndexMap;
use serde::{Deserialize, Serialize};
pub type CategoryId = usize; pub type CategoryId = usize;
pub type ItemId = usize; pub type ItemId = usize;
#[derive(Debug, Clone, Serialize, Deserialize)] #[derive(Debug, Clone)]
pub struct Item { pub struct Item {
pub id: ItemId, pub id: ItemId,
pub name: String, pub name: String,
@@ -27,7 +26,7 @@ impl Item {
} }
} }
#[derive(Debug, Clone, Serialize, Deserialize)] #[derive(Debug, Clone)]
pub struct Group { pub struct Group {
pub name: String, pub name: String,
/// Parent group name for nested hierarchies /// Parent group name for nested hierarchies
@@ -50,7 +49,7 @@ impl Group {
/// Regular categories store their items explicitly; virtual ones are /// Regular categories store their items explicitly; virtual ones are
/// synthesized at query time by the layout layer. context/code/core.md §9. /// 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 { pub enum CategoryKind {
#[default] #[default]
Regular, Regular,
@@ -125,7 +124,7 @@ impl CategoryKind {
} }
} }
#[derive(Debug, Clone, Serialize, Deserialize)] #[derive(Debug, Clone)]
pub struct Category { pub struct Category {
pub id: CategoryId, pub id: CategoryId,
pub name: String, pub name: String,
@@ -136,7 +135,6 @@ pub struct Category {
/// Next item id counter /// Next item id counter
next_item_id: ItemId, next_item_id: ItemId,
/// Whether this is a regular or virtual category /// Whether this is a regular or virtual category
#[serde(default)]
pub kind: CategoryKind, pub kind: CategoryKind,
} }
+1 -37
View File
@@ -7,7 +7,7 @@ use super::symbol::{Symbol, SymbolTable};
/// A cell key: (category, item) pairs, always sorted by category name. The /// 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. /// 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)>); pub struct CellKey(pub Vec<(String, String)>);
impl CellKey { 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)] #[derive(Debug, Clone, Default)]
pub struct DataStore { pub struct DataStore {
/// Primary storage: interned keys, insertion-ordered so records mode can /// Primary storage: interned keys, insertion-ordered so records mode can
@@ -210,29 +208,6 @@ pub struct DataStore {
records: HashMap<RecordHash, RecordEntry>, 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 { impl DataStore {
pub fn new() -> Self { pub fn new() -> Self {
Self::default() Self::default()
@@ -1181,15 +1156,4 @@ mod record_index {
assert_eq!(store.resolve_record(hash), Some(record)); 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);
}
} }
+21 -4
View File
@@ -2,7 +2,6 @@ use std::collections::HashMap;
use anyhow::{Result, anyhow}; use anyhow::{Result, anyhow};
use indexmap::{IndexMap, IndexSet}; use indexmap::{IndexMap, IndexSet};
use serde::{Deserialize, Serialize};
use super::category::{Category, CategoryId, CategoryKind, MEASURE_CATEGORY}; use super::category::{Category, CategoryId, CategoryKind, MEASURE_CATEGORY};
use super::cell::{CellKey, CellValue, DataStore}; use super::cell::{CellKey, CellValue, DataStore};
@@ -300,7 +299,7 @@ impl Aggregation {
/// Pure-data document model: categories, cells, formulas. It knows nothing /// Pure-data document model: categories, cells, formulas. It knows nothing
/// about views — see [`crate::workbook::Workbook`] and core.md §10. /// about views — see [`crate::workbook::Workbook`] and core.md §10.
#[derive(Debug, Clone, Serialize, Deserialize)] #[derive(Clone)]
pub struct Model { pub struct Model {
pub name: String, pub name: String,
pub categories: IndexMap<String, Category>, pub categories: IndexMap<String, Category>,
@@ -309,13 +308,31 @@ pub struct Model {
next_category_id: CategoryId, next_category_id: CategoryId,
/// Per-measure aggregation, used when collapsing `Axis::None` /// Per-measure aggregation, used when collapsing `Axis::None`
/// categories. Defaults to `Sum`. /// categories. Defaults to `Sum`.
#[serde(default)]
pub measure_agg: HashMap<String, AggFunc>, pub measure_agg: HashMap<String, AggFunc>,
/// Cached formula evaluation results. Recomputed by `recompute_formulas`. /// Cached formula evaluation results. Recomputed by `recompute_formulas`.
#[serde(skip)]
formula_cache: HashMap<CellKey, CellValue>, 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 { impl Model {
pub fn new(name: impl Into<String>) -> Self { pub fn new(name: impl Into<String>) -> Self {
let name = name.into(); let name = name.into();
+1 -7
View File
@@ -1,14 +1,9 @@
use indexmap::IndexMap; use indexmap::IndexMap;
use serde::{Deserialize, Serialize};
use std::collections::{HashMap, HashSet}; use std::collections::{HashMap, HashSet};
use super::axis::Axis; use super::axis::Axis;
use crate::model::category::CategoryKind; use crate::model::category::CategoryKind;
fn default_prune() -> bool {
true
}
/// Scratch views the app rebuilds on demand rather than persisting, because /// Scratch views the app rebuilds on demand rather than persisting, because
/// they put the never-persisted `_Index`/`_Dim` on axes (improvise-60z). /// they put the never-persisted `_Index`/`_Dim` on axes (improvise-60z).
pub const VIRTUAL_VIEW_NAMES: [&str; 2] = ["_Records", "_Drill"]; 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") .expect("a name is free below usize::MAX, there being fewer views")
} }
#[derive(Debug, Clone, Serialize, Deserialize)] #[derive(Debug, Clone)]
pub struct View { pub struct View {
pub name: String, pub name: String,
/// Axis assignment for each category /// Axis assignment for each category
@@ -46,7 +41,6 @@ pub struct View {
/// Number format string (e.g. ",.0f" for comma-separated integer) /// Number format string (e.g. ",.0f" for comma-separated integer)
pub number_format: String, pub number_format: String,
/// When true, empty rows/columns are pruned from the display. /// When true, empty rows/columns are pruned from the display.
#[serde(default = "default_prune")]
pub prune_empty: bool, pub prune_empty: bool,
} }
+1 -2
View File
@@ -3,13 +3,12 @@
use anyhow::{Result, anyhow}; use anyhow::{Result, anyhow};
use indexmap::IndexMap; use indexmap::IndexMap;
use serde::{Deserialize, Serialize};
use crate::model::Model; use crate::model::Model;
use crate::model::category::{CategoryId, CategoryKind, ItemId}; use crate::model::category::{CategoryId, CategoryKind, ItemId};
use crate::view::{Axis, DEFAULT_VIEW, View}; use crate::view::{Axis, DEFAULT_VIEW, View};
#[derive(Debug, Clone, Serialize, Deserialize)] #[derive(Debug, Clone)]
pub struct Workbook { pub struct Workbook {
pub model: Model, pub model: Model,
pub views: IndexMap<String, View>, pub views: IndexMap<String, View>,
-1
View File
@@ -10,7 +10,6 @@ repository = "https://github.com/fiddlerwoaroof/improvise"
anyhow = "1" anyhow = "1"
pest = "2.9.2" pest = "2.9.2"
pest_derive = "2.9.2" pest_derive = "2.9.2"
serde = { version = "1", features = ["derive"] }
[dev-dependencies] [dev-dependencies]
pest_meta = "2.9.2" pest_meta = "2.9.2"
+5 -8
View File
@@ -1,6 +1,4 @@
use serde::{Deserialize, Serialize}; #[derive(Debug, Clone)]
#[derive(Debug, Clone, Serialize, Deserialize)]
pub enum AggFunc { pub enum AggFunc {
Sum, Sum,
Avg, Avg,
@@ -11,7 +9,7 @@ pub enum AggFunc {
/// Arithmetic and comparison operators. An enum rather than a `String`, so an /// Arithmetic and comparison operators. An enum rather than a `String`, so an
/// invalid operator is caught at parse time — design-principles §2. /// 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 { pub enum BinOp {
Add, Add,
Sub, Sub,
@@ -26,13 +24,13 @@ pub enum BinOp {
Ge, Ge,
} }
#[derive(Debug, Clone, Serialize, Deserialize)] #[derive(Debug, Clone)]
pub struct Filter { pub struct Filter {
pub category: String, pub category: String,
pub item: String, pub item: String,
} }
#[derive(Debug, Clone, Serialize, Deserialize)] #[derive(Debug, Clone)]
pub enum Expr { pub enum Expr {
Number(f64), Number(f64),
Ref(String), Ref(String),
@@ -45,7 +43,7 @@ pub enum Expr {
Is(Filter), Is(Filter),
} }
#[derive(Debug, Clone, Serialize, Deserialize)] #[derive(Debug, Clone)]
pub struct Formula { pub struct Formula {
/// The raw formula text, e.g. "Profit = Revenue - Cost" /// The raw formula text, e.g. "Profit = Revenue - Cost"
pub raw: String, pub raw: String,
@@ -59,7 +57,6 @@ pub struct Formula {
pub filter: Option<Filter>, pub filter: Option<Filter>,
/// How this formula's target is drawn, overriding the view's `format:` for /// 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`. /// that one measure. `None` means the view answers — `context/code/core.md`.
#[serde(default)]
pub format: Option<String>, pub format: Option<String>,
} }
+1 -1
View File
@@ -13,7 +13,7 @@ anyhow = "1"
chrono = { version = "0.4", features = ["serde"] } chrono = { version = "0.4", features = ["serde"] }
csv = "1" csv = "1"
flate2 = "1" flate2 = "1"
indexmap = { version = "2", features = ["serde"] } indexmap = "2"
pest = "2.9.2" pest = "2.9.2"
pest_derive = "2.9.2" pest_derive = "2.9.2"
serde = { version = "1", features = ["derive"] } serde = { version = "1", features = ["derive"] }
+35 -33
View File
@@ -1,4 +1,4 @@
//! `.improv` document lifecycle — save, load, gzip, legacy JSON — over a //! `.improv` document lifecycle — save, load, gzip — over a
//! [`Storage`] backend. The text format is in `format`, `parse` and `export`. //! [`Storage`] backend. The text format is in `format`, `parse` and `export`.
mod export; mod export;
@@ -96,11 +96,16 @@ pub fn load_from(storage: &dyn Storage, path: &Path) -> Result<Workbook> {
}; };
let text = let text =
String::from_utf8(bytes).with_context(|| format!("{} is not UTF-8", path.display()))?; 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('{') { if text.trim_start().starts_with('{') {
serde_json::from_str(&text).context("Failed to deserialize workbook") anyhow::bail!(
} else { "{} is a legacy JSON workbook, which is no longer supported: \
parse_md(&text) only .improv text can be opened",
path.display()
);
} }
parse_md(&text)
} }
pub fn autosave_path(path: &Path) -> std::path::PathBuf { 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] #[test]
fn load_dispatcher_detects_legacy_json_by_brace() { fn a_legacy_json_workbook_is_refused_by_name() {
// 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() {
use crate::storage::{MemoryFs, Storage}; use crate::storage::{MemoryFs, Storage};
const LOSSY: f64 = 0.248_157_142_857_142_85; // The shape the removed loader read: `Workbook`'s serde form.
let key = coord(&[("Type", "Food"), ("Month", "Jan")]); let legacy = r#"
let mut wb = two_cat_model(); {"model": {"name": "Budget", "categories": {}, "data": [], "formulas": []},
wb.model.set_cell(key.clone(), CellValue::Number(LOSSY)); "views": {}, "active_view": "Default"}"#;
let json = serde_json::to_string(&wb).unwrap(); for name in ["legacy.improv", "legacy.improv.gz"] {
assert!( let fs = MemoryFs::new();
json.contains("0.24815714285714285"), let path = std::path::Path::new(name);
"the file holds the exact text" let bytes = if name.ends_with(".gz") {
); use std::io::Write;
let fs = MemoryFs::new(); let mut enc =
let path = std::path::Path::new("legacy.improv"); flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::default());
fs.write(path, json.as_bytes()).unwrap(); enc.write_all(legacy.as_bytes()).unwrap();
let loaded = super::load_from(&fs, path).unwrap(); enc.finish().unwrap()
let Some(CellValue::Number(n)) = loaded.model.get_cell(&key) else { } else {
panic!("the cell must load as a number"); legacy.as_bytes().to_vec()
}; };
assert_eq!(n.to_bits(), LOSSY.to_bits(), "loaded {n:?}, not {LOSSY:?}"); 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 ──────────────────────────────────── // ── save/load roundtrip via file ────────────────────────────────────
-3
View File
@@ -196,7 +196,6 @@ dependencies = [
"anyhow", "anyhow",
"pest", "pest",
"pest_derive", "pest_derive",
"serde",
] ]
[[package]] [[package]]
@@ -228,8 +227,6 @@ checksum = "cc4e190f5d26ca7051642629da2c52fc03bde85a03197c99408dcd291734c855"
dependencies = [ dependencies = [
"equivalent", "equivalent",
"hashbrown", "hashbrown",
"serde",
"serde_core",
] ]
[[package]] [[package]]