Merge pull request #129 from fiddlerwoaroof/fix/label-and-views
CI / Build all bundles (push) Has been cancelled
CI / Test and lint (host) (push) Has been cancelled
CI / wasm32 check (browser bundle crates) (push) Has been cancelled
CI / Browser JS unit tests (node --test) (push) Has been cancelled
CI / Knowledge base consistency (push) Has been cancelled
CI / Lint the harness crate (push) Has been cancelled
CI / Browser harness (OPFS contract + shell) (push) Has been cancelled
CI / Deploy to GitHub Pages (push) Has been cancelled

fix: a Label category survives a save, and a new view does not pivot it
This commit is contained in:
Ed Langley
2026-09-22 01:31:52 -07:00
committed by GitHub
12 changed files with 391 additions and 40 deletions
+30
View File
@@ -463,6 +463,36 @@ auto Row/Column assignment.
`Category::add_item` deduplicates by name and assigns IDs from a private
counter, so external code cannot fabricate an `ItemId`.
### Seating takes the kind, because a name cannot carry it (improvise-cj4u)
`View::on_category_added` takes a `CategoryKind` beside the name. It used to
take the name alone and ask `of_name`, which reads `Label` as `Regular` by
design — so the *default* view got a label on `Axis::None` (because
`Workbook::add_label_category` set it explicitly afterwards) while
`Workbook::create_view`, which only ever had names to hand, seated the same
label on Row. A high-cardinality description column therefore became a pivot
axis in every view created after it, and on an already-pivoted document it
landed on `Page`, filtering the whole document down to one description.
Two things follow, and the second is the one worth carrying.
**The parameter is the fix, not a `set_axis` after the fact.** The old
`add_label_category` seated the category and then corrected it, which is two
answers to "where does this category go" — and only one of the two callers of
`on_category_added` knew to write the second. The correction is now deleted:
seating is told the kind and answers once. `add_label_category_sets_none_axis_
on_all_views` and `label_category_defaults_to_none_axis` both still pass, and
both fail if the parameter is ignored, so the consolidation kept its evidence.
**`of_name` survives, narrowed.** Every remaining caller either holds no
`Category` (the view's own `is_records_layout`, `holds_a_regular`) or is test
code building a view from bare names. That residue is real: `holds_a_regular`
asks of the categories *already seated* on an axis whether one of them pivots,
and it asks by name, so a label a user deliberately moved onto Row still counts
as occupying it. That is the right answer — a category the user put on Row is
pivoting there — but it is an answer reached by a different route from the one
above, and a future `View` that stores kinds should collapse the two.
### Removing a name removes the formulas targeting it (improvise-g3no)
`Model::remove_category` has always ended by dropping every formula whose
+64
View File
@@ -154,6 +154,70 @@ non-atomic rules produce the whitespace-separated tokens pest's implicit
with three or more alternatives (nested `Choice`s) pick deeper branches less
often — good enough for random generation, not a uniform distribution.
### The category kind is a line, not a header suffix (improvise-dy3j)
`CategoryKind::Label` had no spelling in the format at all. `format_md` wrote
every category as a bare `## Category: Name` and `build_category` read every
section back through `add_category`, so the kind was dropped on the way out
**and** on the way in. A round-trip equality test cannot see a loss both sides
share, which is why the existing suite was silent — the same shape as the
`Formula::raw` traps above, and the reason the new tests assert the *parsed*
`Category::kind` rather than the emitted text.
The consequence was a lost document, not a lost flag. `Label` sits outside
`MAX_CATEGORIES = 12` on purpose, and the import wizard's own precondition
scores a label as zero, so it will happily build twelve categories plus a
label. On load every label came back `Regular`, the thirteenth was refused, and
`parse_md` failed: **any document where regular + label exceeded twelve was
permanently unloadable.** Below the limit the failure was quieter — a label
silently became a pivot category, entering `regular_category_names()` pickers
and gaining axis auto-assignment.
**The ticket proposed `## Category: Note [label]`, and the grammar forbids
it.** A section header takes its name as the **whole line**, totally
(improvise-pczv) — `header_name` is `pipe_quoted ~ &NEWLINE | line_name` — so a
suffix is not a suffix, it is part of a category literally named `Note
[label]`. The two readings are indistinguishable, and adopting the suffix would
have made a legal name unspellable while quietly re-opening the ambiguity
`header_name` exists to close.
So the kind is an **entry inside the section**, beside the item lines:
```
## Category: Note
kind: label
- see the record view
```
Three properties earned it. It uses a **real token** (`"kind: "`), not a
lookahead, because the grammar-walking generator reads the grammar through
`pest_meta` and *skips predicates* — a rule guarded only by a lookahead is a
rule the generator can violate, which is why `WHERE` carries `sp+`. It cannot
collide with `item_list`, `grouped_item` or `group_hierarchy`, each of which
needs a `- ` or `> ` lead. And **absence is the compatible reading**: `kind_kw`
spells only `label`, the writer emits the line only for a label, so every file
written before this commit parses unchanged and every pivot category's section
is byte-identical to what it was. That is also why `parse_kind_kw` is an
exhaustive match returning `Result` rather than a `bool` read off the rule's
mere presence — a second keyword must fail loudly at the walker, not be read as
`label` because it arrived on a `kind_line`.
One case the format already allowed and this had to answer: two
`## Category:` sections naming one category are one category (improvise-349m),
merged in pass 1. Absence is not a claim, so the merge keeps a stated kind
rather than letting a later kindless section erase it.
**The generator coverage this section demands was measured, not assumed.**
`kind_line` is an alternative of `category_entry`, which the grammar walk
reaches generically, so nothing in the generator needed changing — but that is
the kind of claim this section exists to say must be checked. Probed with a
throwaway test over 500 files from `generator::improv_file()`: **14 carried a
`kind: label` line and all 14 parsed to a `CategoryKind::Label`**, so at the
default 256 cases each proptest run exercises the new production about seven
times. The probe was removed rather than kept: its rate depends on proptest's
internal RNG, so as a permanent assertion it would fail on a dependency bump
for a reason unrelated to the format.
### The tree walkers are infallible by construction
Every `build_*` in `parser.rs` operates on a `Pair` pest has already
+43
View File
@@ -1143,3 +1143,46 @@ luck. It is the same shape as `analyze_records`' five-iteration guard.
This one cannot be made to fail by re-introducing a `HashMap` *reliably*, which
is why the test that carries the load is its sibling above; kept because the
repetition is what documents that hash order was the defect.
### a_view_created_after_a_label_still_keeps_it_off_the_axes
`View::on_category_added` took a name and asked `CategoryKind::of_name`, which
reads `Label` as `Regular` by design. `Workbook::add_label_category` papered
over that by calling `set_axis(name, Axis::None)` straight afterwards, so the
views existing *at that moment* were right; `Workbook::create_view` had only
names to hand and had no such correction, so every view created after the label
pivoted it. Reproduced at `d46ec59`: `Description` on `Axis::Row` in a view
created one line after `add_label_category`.
The fix gives seating the kind, and the two `set_axis` corrections are deleted —
one answer rather than two, of which only one caller knew to write the second.
### a_view_created_after_a_regular_category_still_seats_it_on_row
The control, and the direction the bug's own shape cannot supply. Seating
everything on `Axis::None` satisfies the label assertion above completely, so
without this a blanket `unseated = true` reads as a fix.
Fault injected: `let unseated = true;`. This test fails (`left: None right:
Row`) along with `a_label_does_not_displace_a_regular_category_in_a_new_view`
and `first_category_goes_to_row_second_to_column_rest_to_page`, while every
label assertion in the repository still passes.
### a_label_does_not_displace_a_regular_category_in_a_new_view
The consequence the per-category assertions cannot state, because seating is
**order-dependent**: the first regular category takes Row, the second Column,
the rest Page. A label wrongly counted as regular therefore does not merely sit
in the wrong place — it *consumes the slot* the next real category wanted, and
on an already-pivoted document it pushes that category to `Page`, which
filters the whole document down to one value.
The fixture adds the label first and the regular category second, so the
displacement is what is measured rather than the label's own axis.
Fault injected: restoring `let unseated = CategoryKind::of_name(cat_name)
.is_virtual() || self.is_records_layout();`. Five tests fail, including the two
pre-existing ones (`add_label_category_sets_none_axis_on_all_views`,
`label_category_defaults_to_none_axis`) that used to be carried by the deleted
`set_axis` correction — which is the evidence that the consolidation did not
cost coverage.
+66
View File
@@ -1288,3 +1288,69 @@ the parse would fail the test for the wrong reason.
The absent case, so the field above is something the file carried rather than a
constant the walker invents. Without it, a walker hard-coding `Some(".4")`
would pass the test above.
### a_label_category_survives_the_round_trip_as_a_label
`CategoryKind::Label` had no spelling in `.improv` at all: `format_md` wrote
every category as a bare `## Category: Name` and `build_category` read every
section back through `Workbook::add_category`. The kind was dropped on the way
out *and* on the way in, so the loss was symmetric and no round-trip equality
test could see it — the same shape the `Formula::format` entry above records.
Reproduced at `d46ec59`: a workbook with `Region` (regular) and `Note` (label)
came back with `Note.kind == Regular`.
The test asserts the **parsed** `Category::kind`, never the emitted line, for
that reason. Its second assertion — that `Region` still reads back `Regular` —
is the control: a reader that made every category a `Label` satisfies the first
assertion and fails this one.
Faults injected, both against the fixed tree:
- **Writer drops the line** (`if false` around `w!(out, "kind: label")`). Fails
`left: Regular right: Label`, and takes
`a_document_at_the_category_limit_plus_a_label_reloads` with it.
- **Reader ignores the kind it parsed** (`build_category` calls `add_category`
unconditionally, discarding `pc.kind`). Same two failures. This is the
discriminating one: the file text is *correct* in this variant, so a test
comparing emitted output passes.
### a_document_at_the_category_limit_plus_a_label_reloads
The P1 half, and the reason improvise-dy3j was not merely a lost flag. `Label`
is outside `MAX_CATEGORIES = 12` by design — `Model::add_label_category` has no
limit check, and the import wizard's `proposed_category_count` scores a label
as zero — so a document with twelve categories and one label is legally
constructible and the wizard will build one from any CSV carrying a free-text
column. Every label came back `Regular` on load, the thirteenth category was
refused, and `parse_md` returned
`Err("Maximum of 12 categories reached")`: **the document was permanently
unloadable.** Reproduced at `d46ec59` with exactly that error.
The second assertion (`regular_category_names().len() == 12`) is what keeps the
first honest. A "fix" that simply raised or removed the limit would let the
document load while putting a high-cardinality label into the pivot count, and
this assertion fails on it.
### a_category_section_with_no_kind_line_loads_as_regular
The absent case: every `.improv` file written before this commit carries no
`kind:` line, and must still load. Without it, a walker that defaulted
`PCategory::kind` to `Label` would pass both tests above.
The input is hand-written text rather than a round trip, deliberately — a round
trip cannot exhibit an absent line, because the writer under test is the thing
that would have written it.
### a_loaded_label_category_is_not_on_a_pivot_axis
The end-to-end check that the two halves of improvise-dy3j/cj4u agree: a label
is off every pivot axis in every view after a save and a load, including a view
created after the label.
Worth stating what it does and does not witness. It is **not** a second witness
for the kind: `format_md` writes each view's axes explicitly, so `Note: none`
carries the seating across the file whatever the kind does. What it witnesses
is the *seating* — with the `improvise-cj4u` fix reverted, `create_view`
pivots `Note` onto Column in the source workbook, the file faithfully records
`Note: column`, and this test fails. It was written while cj4u was still open
and failed then, which is how that was established rather than assumed.
+21 -20
View File
@@ -72,13 +72,12 @@ impl View {
.any(|c| CategoryKind::of_name(c).is_dim())
}
/// Seat a newly added category: a virtual on `None`, a regular one on the
/// first slot no regular holds. A virtual is **never** bumped (§7).
pub fn on_category_added(&mut self, cat_name: &str) {
/// Seat a newly added category: only a `Regular` one takes a pivot slot,
/// and the kind is the caller's because a name cannot carry it (§7).
pub fn on_category_added(&mut self, cat_name: &str, kind: CategoryKind) {
if !self.category_axes.contains_key(cat_name) {
let virtual_or_records =
CategoryKind::of_name(cat_name).is_virtual() || self.is_records_layout();
let axis = if virtual_or_records {
let unseated = !kind.is_regular() || self.is_records_layout();
let axis = if unseated {
Axis::None
} else {
let holds_a_regular = |axis: Axis| {
@@ -258,12 +257,13 @@ impl View {
#[cfg(test)]
mod tests {
use super::View;
use crate::model::category::CategoryKind;
use crate::view::Axis;
fn view_with_cats(cats: &[&str]) -> View {
let mut v = View::new("Test");
for &c in cats {
v.on_category_added(c);
v.on_category_added(c, CategoryKind::of_name(c));
}
v
}
@@ -289,7 +289,7 @@ mod tests {
v.set_axis("Region", Axis::None);
assert!(v.is_records_layout(), "setup");
v.on_category_added("Product");
v.on_category_added("Product", CategoryKind::of_name("Product"));
assert_eq!(v.axis_of("Product"), Axis::None);
assert_eq!(v.axis_of("_Index"), Axis::Row, "_Index must not be bumped");
assert_eq!(v.axis_of("_Dim"), Axis::Column, "_Dim must not be bumped");
@@ -305,7 +305,7 @@ mod tests {
assert_eq!(v.axis_of("Region"), Axis::Row, "setup");
assert!(!v.is_records_layout(), "setup: a pivot view, not records");
v.on_category_added("Product");
v.on_category_added("Product", CategoryKind::of_name("Product"));
assert_eq!(
v.axis_of("_Measure"),
Axis::Column,
@@ -538,10 +538,10 @@ mod tests {
#[test]
fn on_category_removed_forgets_a_collapsed_group() {
let mut v = View::new("V");
v.on_category_added("Month");
v.on_category_added("Month", CategoryKind::of_name("Month"));
v.toggle_group_collapse("Month", "Q1");
v.on_category_removed("Month");
v.on_category_added("Month");
v.on_category_added("Month", CategoryKind::of_name("Month"));
assert!(
!v.is_group_collapsed("Month", "Q1"),
"a re-added category came back with its group collapsed"
@@ -553,7 +553,7 @@ mod tests {
#[test]
fn on_item_removed_forgets_a_hidden_item() {
let mut v = View::new("V");
v.on_category_added("Region");
v.on_category_added("Region", CategoryKind::of_name("Region"));
v.hide_item("Region", "East");
v.hide_item("Region", "West");
v.on_item_removed("Region", "East");
@@ -569,7 +569,7 @@ mod tests {
#[test]
fn on_item_removed_forgets_a_page_selection_naming_it() {
let mut v = View::new("V");
v.on_category_added("Region");
v.on_category_added("Region", CategoryKind::of_name("Region"));
v.set_page_selection("Region", "East");
v.on_item_removed("Region", "West");
assert_eq!(
@@ -625,6 +625,7 @@ mod tests {
#[cfg(test)]
mod prop_tests {
use super::View;
use crate::model::category::CategoryKind;
use crate::view::Axis;
use proptest::prelude::*;
@@ -638,7 +639,7 @@ mod prop_tests {
#[test]
fn axis_of_and_categories_on_consistent(cats in unique_cat_names()) {
let mut v = View::new("T");
for c in &cats { v.on_category_added(c); }
for c in &cats { v.on_category_added(c, CategoryKind::of_name(c)); }
for c in &cats {
let axis = v.axis_of(c);
let on_axis = v.categories_on(axis);
@@ -651,7 +652,7 @@ mod prop_tests {
#[test]
fn each_category_on_exactly_one_axis(cats in unique_cat_names()) {
let mut v = View::new("T");
for c in &cats { v.on_category_added(c); }
for c in &cats { v.on_category_added(c, CategoryKind::of_name(c)); }
let all_axes = [Axis::Row, Axis::Column, Axis::Page, Axis::None];
for c in &cats {
let count = all_axes.iter()
@@ -666,9 +667,9 @@ mod prop_tests {
#[test]
fn on_category_added_idempotent(cats in unique_cat_names()) {
let mut v = View::new("T");
for c in &cats { v.on_category_added(c); }
for c in &cats { v.on_category_added(c, CategoryKind::of_name(c)); }
let axes_before: Vec<Axis> = cats.iter().map(|c| v.axis_of(c)).collect();
for c in &cats { v.on_category_added(c); }
for c in &cats { v.on_category_added(c, CategoryKind::of_name(c)); }
let axes_after: Vec<Axis> = cats.iter().map(|c| v.axis_of(c)).collect();
prop_assert_eq!(axes_before, axes_after);
}
@@ -681,7 +682,7 @@ mod prop_tests {
axis in prop_oneof![Just(Axis::Row), Just(Axis::Column), Just(Axis::Page), Just(Axis::None)],
) {
let mut v = View::new("T");
for c in &cats { v.on_category_added(c); }
for c in &cats { v.on_category_added(c, CategoryKind::of_name(c)); }
let idx = target_idx % cats.len();
let cat = &cats[idx];
v.set_axis(cat, axis);
@@ -696,7 +697,7 @@ mod prop_tests {
axis in prop_oneof![Just(Axis::Row), Just(Axis::Column), Just(Axis::Page), Just(Axis::None)],
) {
let mut v = View::new("T");
for c in &cats { v.on_category_added(c); }
for c in &cats { v.on_category_added(c, CategoryKind::of_name(c)); }
let idx = target_idx % cats.len();
let cat = &cats[idx];
v.set_axis(cat, axis);
@@ -714,7 +715,7 @@ mod prop_tests {
#[test]
fn total_category_count_consistent(cats in unique_cat_names()) {
let mut v = View::new("T");
for c in &cats { v.on_category_added(c); }
for c in &cats { v.on_category_added(c, CategoryKind::of_name(c)); }
let total: usize = [Axis::Row, Axis::Column, Axis::Page]
.iter()
.map(|&ax| v.categories_on(ax).len())
+43 -8
View File
@@ -6,7 +6,7 @@ use indexmap::IndexMap;
use serde::{Deserialize, Serialize};
use crate::model::Model;
use crate::model::category::{CategoryId, ItemId};
use crate::model::category::{CategoryId, CategoryKind, ItemId};
use crate::view::{Axis, DEFAULT_VIEW, View};
#[derive(Debug, Clone, Serialize, Deserialize)]
@@ -29,8 +29,8 @@ impl Workbook {
active_view: DEFAULT_VIEW.to_string(),
};
for view in wb.views.values_mut() {
for cat_name in wb.model.categories.keys() {
view.on_category_added(cat_name);
for cat in wb.model.categories.values() {
view.on_category_added(&cat.name, cat.kind);
}
view.set_axis("_Measure", Axis::Page);
}
@@ -44,7 +44,7 @@ impl Workbook {
let name = name.into();
let id = self.model.add_category(&name)?;
for view in self.views.values_mut() {
view.on_category_added(&name);
view.on_category_added(&name, CategoryKind::Regular);
}
Ok(id)
}
@@ -55,8 +55,7 @@ impl Workbook {
let name = name.into();
let id = self.model.add_label_category(&name)?;
for view in self.views.values_mut() {
view.on_category_added(&name);
view.set_axis(&name, Axis::None);
view.on_category_added(&name, CategoryKind::Label);
}
Ok(id)
}
@@ -123,8 +122,8 @@ impl Workbook {
pub fn create_view(&mut self, name: impl Into<String>) -> &mut View {
let name = name.into();
let mut view = View::new(name.clone());
for cat_name in self.model.categories.keys() {
view.on_category_added(cat_name);
for cat in self.model.categories.values() {
view.on_category_added(&cat.name, cat.kind);
}
self.views.insert(name.clone(), view);
self.views.get_mut(&name).unwrap()
@@ -187,6 +186,42 @@ mod tests {
assert_eq!(wb.views.get("Other").unwrap().axis_of("Note"), Axis::None);
}
/// A view created *after* the label must seat it off the pivot axes too.
/// Evidence: `context/test-evidence/core.md`.
#[test]
fn a_view_created_after_a_label_still_keeps_it_off_the_axes() {
let mut wb = Workbook::new("Test");
wb.add_label_category("Description").unwrap();
assert_eq!(wb.active_view().axis_of("Description"), Axis::None);
assert_eq!(wb.create_view("Other").axis_of("Description"), Axis::None);
}
/// The control for the test above: a regular category added the same way
/// still takes the first free pivot slot, so `None` is not the blanket answer.
#[test]
fn a_view_created_after_a_regular_category_still_seats_it_on_row() {
let mut wb = Workbook::new("Test");
wb.add_category("Region").unwrap();
assert_eq!(wb.create_view("Other").axis_of("Region"), Axis::Row);
}
/// A label must not consume the Row slot a later regular category wants.
/// Evidence: `context/test-evidence/core.md`.
#[test]
fn a_label_does_not_displace_a_regular_category_in_a_new_view() {
let mut wb = Workbook::new("Test");
wb.add_label_category("Description").unwrap();
wb.add_category("Region").unwrap();
let v = wb.create_view("Other");
assert_eq!(v.axis_of("Description"), Axis::None);
assert_eq!(
v.axis_of("Region"),
Axis::Row,
"Row was free — the label must not have taken it"
);
}
#[test]
fn remove_category_removes_from_all_views() {
let mut wb = Workbook::new("Test");
+1 -1
View File
@@ -257,7 +257,7 @@ mod tests {
let mut view = View::new("Default");
for cat in model.category_names() {
view.on_category_added(cat);
view.on_category_added(cat, CategoryKind::of_name(cat));
}
for cat in [QUARTER, NOTE] {
view.set_axis(cat, Axis::None);
+7 -7
View File
@@ -502,7 +502,7 @@ mod tests {
}
let mut view = View::new("Default");
for cat in model.category_names() {
view.on_category_added(cat);
view.on_category_added(cat, CategoryKind::of_name(cat));
}
view.set_axis(REGION, Axis::Row);
// `Workbook::new` — the constructor a real document comes from — leaves
@@ -714,7 +714,7 @@ mod tests {
}
let mut view = View::new("Default");
for cat in model.category_names() {
view.on_category_added(cat);
view.on_category_added(cat, CategoryKind::of_name(cat));
}
view.set_axis("Year", Axis::Row);
view.set_axis(MEASURE, Axis::Page);
@@ -826,7 +826,7 @@ mod tests {
}
let mut view = View::new("Default");
for cat in model.category_names() {
view.on_category_added(cat);
view.on_category_added(cat, CategoryKind::of_name(cat));
}
view.set_axis("Wide", Axis::Row);
view.set_axis(MEASURE, Axis::Page);
@@ -872,7 +872,7 @@ mod tests {
}
let mut view = View::new("Default");
for cat in model.category_names() {
view.on_category_added(cat);
view.on_category_added(cat, CategoryKind::of_name(cat));
}
view.set_axis("Year", Axis::Row);
view.set_axis(MEASURE, Axis::Page);
@@ -962,7 +962,7 @@ mod tests {
}
let mut view = View::new("Default");
for cat in model.category_names() {
view.on_category_added(cat);
view.on_category_added(cat, CategoryKind::of_name(cat));
}
view.set_axis(REGION, Axis::Row);
view.set_axis(MEASURE, Axis::Page);
@@ -1050,7 +1050,7 @@ mod tests {
}
let mut view = View::new("Default");
for cat in model.category_names() {
view.on_category_added(cat);
view.on_category_added(cat, CategoryKind::of_name(cat));
}
view.set_axis("Year", Axis::Row);
view.set_axis(MEASURE, Axis::Page);
@@ -1122,7 +1122,7 @@ mod tests {
}
let mut view = View::new("Default");
for cat in model.category_names() {
view.on_category_added(cat);
view.on_category_added(cat, CategoryKind::of_name(cat));
}
view.set_axis("Dup", Axis::Row);
view.set_axis(MEASURE, Axis::Page);
@@ -181,6 +181,11 @@ pub fn format_md(workbook: &Workbook) -> String {
continue;
}
w!(out, "\n## Category: {}", quote_header(&cat.name));
// The kind is written only where it is not the default, so a pivot
// category's section is byte-identical to before (improvise-dy3j).
if cat.kind == CategoryKind::Label {
w!(out, "kind: label");
}
// Items are written in insertion order, because that is the order the
// grid's axes consume (gotcha 18). A run of bare items shares a line.
let mut bare: Vec<String> = Vec::new();
@@ -36,7 +36,12 @@ category_section = {
category_entry*
}
category_entry = _{ group_hierarchy | grouped_item | item_list }
category_entry = _{ kind_line | group_hierarchy | grouped_item | item_list }
// The category's kind. Absent means `regular` — the compatible reading of
// every file written before this line existed. context/code/formula-and-io.md.
kind_line = { "kind: " ~ kind_kw ~ NEWLINE ~ blank_lines }
kind_kw = @{ "label" }
// Comma-separated bare items (no group): `- Food, Gas, Total`
item_list = {
+28 -2
View File
@@ -7,7 +7,7 @@ use pest::iterators::{Pair, Pairs};
use pest_derive::Parser;
use crate::formula::{AggFunc, parse_formula};
use crate::model::category::Group;
use crate::model::category::{CategoryKind, Group};
use crate::model::cell::{CellKey, CellValue};
use crate::view::Axis;
use crate::workbook::Workbook;
@@ -27,6 +27,9 @@ struct PCategory {
/// is load-bearing, so pass 2 must replay this list as written.
items: Vec<(String, Option<String>)>,
group_parents: Vec<(String, String)>,
/// What a `kind:` line said, `Regular` where none did. Absence is not a
/// claim, so merging two sections keeps the stated one — context/code/formula-and-io.md.
kind: CategoryKind,
}
/// One `## View:` section as pass 1 read it.
@@ -60,16 +63,21 @@ impl ParsedFile {
name,
items,
group_parents,
kind,
} = pc;
match self.categories.iter_mut().find(|e| e.name == name) {
Some(existing) => {
existing.items.extend(items);
existing.group_parents.extend(group_parents);
if kind != CategoryKind::Regular {
existing.kind = kind;
}
}
None => self.categories.push(PCategory {
name,
items,
group_parents,
kind,
}),
}
}
@@ -164,9 +172,14 @@ fn walk_category_section(pair: Pair<'_, Rule>) -> Result<PCategory> {
name: section_title(&mut inner, "category_section")?,
items: Vec::new(),
group_parents: Vec::new(),
kind: CategoryKind::Regular,
};
for entry in inner {
match entry.as_rule() {
Rule::kind_line => {
let kw = next(&mut entry.into_inner(), "kind_line")?;
pc.kind = parse_kind_kw(kw.as_str())?;
}
Rule::item_list => {
for name_pair in entry.into_inner() {
pc.items.push((extract_name(name_pair)?, None));
@@ -185,6 +198,15 @@ fn walk_category_section(pair: Pair<'_, Rule>) -> Result<PCategory> {
Ok(pc)
}
/// The on-disk keyword for a category kind, exhaustively — the reader's half
/// of the `kind: label` line `format.rs` writes.
fn parse_kind_kw(kind: &str) -> Result<CategoryKind> {
match kind {
"label" => Ok(CategoryKind::Label),
_ => bail!("Unknown category kind: {kind}"),
}
}
/// The on-disk keyword for an aggregation, exhaustively — the reader's half of
/// `format.rs::agg_keyword`.
fn parse_agg_kind(kind: &str) -> Result<AggFunc> {
@@ -361,7 +383,10 @@ fn walk_file(file: Pair<'_, Rule>) -> Result<ParsedFile> {
/// Register one category's items and groups, in file order.
fn build_category(wb: &mut Workbook, pc: &PCategory) -> Result<()> {
wb.add_category(&pc.name)?;
match pc.kind {
CategoryKind::Label => wb.add_label_category(&pc.name)?,
_ => wb.add_category(&pc.name)?,
};
let cat = wb
.model
.category_mut(&pc.name)
@@ -490,6 +515,7 @@ mod grammar_coverage {
Rule::section => "silent: hoisted into file",
Rule::category_section => "walk_category_section",
Rule::item_list | Rule::grouped_item | Rule::group_hierarchy => "walk_category_section",
Rule::kind_line | Rule::kind_kw => "walk_category_section",
Rule::category_entry => "silent: hoisted into category_section",
Rule::formulas_section | Rule::formula_line | Rule::formula_raw => {
"walk_formulas_section"
@@ -4,7 +4,7 @@
use super::test_helpers::{coord, two_cat_model};
use super::{format_md, parse_md};
use crate::formula::parse_formula;
use crate::model::category::Group;
use crate::model::category::{CategoryKind, Group};
use crate::model::cell::CellValue;
use crate::view::Axis;
use crate::workbook::Workbook;
@@ -1158,3 +1158,79 @@ fn parse_md_reads_a_formula_with_no_format_as_none() {
let loaded = parse_md(&format_md(&m)).unwrap();
assert_eq!(loaded.model.formulas()[0].format, None);
}
// ── Category kinds (improvise-dy3j) ──────────────────────────────────────
/// The kind is a parsed field, not a spelling the file happens to carry.
/// Evidence: `context/test-evidence/formula-and-io.md`.
#[test]
fn a_label_category_survives_the_round_trip_as_a_label() {
let mut wb = Workbook::new("T");
wb.add_category("Region").unwrap();
wb.add_label_category("Note").unwrap();
let loaded = parse_md(&format_md(&wb)).unwrap();
assert_eq!(
loaded.model.category("Note").unwrap().kind,
CategoryKind::Label
);
// The control that keeps the assertion above from passing vacuously: a
// reader making every category a Label would clear it and fail here.
assert_eq!(
loaded.model.category("Region").unwrap().kind,
CategoryKind::Regular
);
}
/// A label sits outside `MAX_CATEGORIES`, so a document at the limit plus one
/// label must reload. Evidence: `context/test-evidence/formula-and-io.md`.
#[test]
fn a_document_at_the_category_limit_plus_a_label_reloads() {
let mut wb = Workbook::new("T");
for i in 0..12 {
wb.add_category(format!("C{i}")).unwrap();
}
wb.add_label_category("Note").unwrap();
let loaded = parse_md(&format_md(&wb)).expect("a label must not count against the limit");
assert_eq!(
loaded.model.category("Note").unwrap().kind,
CategoryKind::Label
);
assert_eq!(loaded.model.regular_category_names().len(), 12);
}
/// A file written before the `kind:` line existed reads its categories as
/// `Regular` — the compatible reading, and what the writer still omits.
#[test]
fn a_category_section_with_no_kind_line_loads_as_regular() {
let text = "v2025-04-09\n\
# Budget\n\
\n\
## Category: Region\n\
- North, South\n";
let loaded = parse_md(text).unwrap();
assert_eq!(
loaded.model.category("Region").unwrap().kind,
CategoryKind::Regular
);
}
/// The loader seats a label off every pivot axis, which is the view half of
/// the same fact (improvise-cj4u).
#[test]
fn a_loaded_label_category_is_not_on_a_pivot_axis() {
let mut wb = Workbook::new("T");
wb.add_category("Region").unwrap();
wb.add_label_category("Note").unwrap();
wb.create_view("Other");
let loaded = parse_md(&format_md(&wb)).unwrap();
for (name, view) in &loaded.views {
assert_eq!(
view.axis_of("Note"),
Axis::None,
"view {name} pivoted a label category"
);
}
}