feat(core): raise MAX_CATEGORIES from 12 to 24, with one home (improvise-felo.1)

The wizard's private copy of the limit is gone; it reads the model's
now-public constant. Tests that fill to the cap derive it. Why 12 was
chosen and what 24 costs: context/code/core.md section 9.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
Edward Langley
2026-09-24 19:16:04 -07:00
parent bb01cd1957
commit 7d1f90dbf9
17 changed files with 240 additions and 42 deletions
+1 -1
View File
@@ -102,7 +102,7 @@ rollback target — it is not what the published site is running.
Every cell lives at the intersection of named categories and items — Every cell lives at the intersection of named categories and items —
`(Region=East, Measure=Revenue)` — not at a grid address like `B3`. A model `(Region=East, Measure=Revenue)` — not at a grid address like `B3`. A model
can have up to 12 categories, each with an ordered list of items that can be can have up to 24 categories, each with an ordered list of items that can be
organized into collapsible groups (e.g. months grouped into quarters). organized into collapsible groups (e.g. months grouped into quarters).
Formulas reference dimension names, not cell addresses: Formulas reference dimension names, not cell addresses:
+1 -1
View File
@@ -831,7 +831,7 @@ The classes that remain:
column renumbers every index to its right and the advance was computed from column renumbers every index to its right and the advance was computed from
the pre-commit layout (gotcha 23), so `CommitAndAdvance` returns straight the pre-commit layout (gotcha 23), so `CommitAndAdvance` returns straight
after it and a client that advanced anyway would land one column off, after it and a client that advanced anyway would land one column off,
silently. `AddCategory` can also be refused by the 12-category limit and sets silently. `AddCategory` can also be refused by the category limit and sets
`abort_effects`; the mode change therefore goes **first**, where an abort `abort_effects`; the mode change therefore goes **first**, where an abort
cannot reach it (gotcha 21). **Since improvise-bd9o that exception is cannot reach it (gotcha 21). **Since improvise-bd9o that exception is
unreachable**, and this bullet describes `commit` alone: the records decline unreachable**, and this bullet describes `commit` alone: the records decline
+52 -1
View File
@@ -518,12 +518,63 @@ explicitly would be redundant in the file format and confusing in the UI.
`Label` is a high-cardinality per-row field (description, id, note): stored `Label` is a high-cardinality per-row field (description, id, note): stored
alongside the data so it shows up in record/drill views, but defaulting to alongside the data so it shows up in record/drill views, but defaulting to
`Axis::None` and excluded from the `MAX_CATEGORIES = 12` pivot limit and from `Axis::None` and excluded from the `MAX_CATEGORIES` pivot limit and from
auto Row/Column assignment. auto Row/Column assignment.
`Category::add_item` deduplicates by name and assigns IDs from a private `Category::add_item` deduplicates by name and assigns IDs from a private
counter, so external code cannot fabricate an `ItemId`. counter, so external code cannot fabricate an `ItemId`.
### The limit is 24, and nothing is exponential in it (improvise-felo.1)
`MAX_CATEGORIES` counts **regular** categories only. It is `pub` in
`model/types.rs` and re-exported as `improvise_core::model::MAX_CATEGORIES`,
and it has one home: the import wizard's precondition reads it rather than
keeping a copy. It had a copy until felo.1, a private `const` of its own, and
raising the model's number alone would have left the wizard refusing a 13th
category the model accepts. Tests that fill to the cap derive it from the
constant; `add_category_max_limit` is the one place the number 24 is spelled.
**Why 12.** Nothing recorded says. The limit arrived in the initial commit
with no comment, ADR or ticket behind it, and nothing in `context/` or `kb.pl`
argues for it. The likeliest source is Lotus Improv, which this tool follows
by name (`context/plan.md`) and whose worksheets are remembered as capped at
12 categories. That is not verified here. It was never a cost bound. The user raised it on 2026-09-25: "12 categories is
feeling small, we should increase to 24". An E*Trade import hit exactly 12,
leaving no room for improvise-cgqt's disambiguating category.
**What scales with the number of categories**, read off the code:
- `CellKey` is a sorted `Vec` of pairs: building one is O(k log k), and
comparing or hashing one is O(k).
- The `DataStore` secondary index holds one `(category, item)` entry per
coordinate per cell. That is O(k) per insert and O(cells × k) in memory.
- Records mode has one column per regular category, so it is linear in k.
- `cross_product` is the product of the item counts of the categories **on
an axis**. That is exponential in what the user places on the axis, and the
cap never bounded it: 12 categories of 4 items on rows is already 16M rows.
`prune_empty` is what keeps it drawable.
- `improvise-gp` bounds its encoded width separately (`MAX_FEATURES = 64`),
so a wide fit is refused by name rather than slowed.
**Measured**, `cargo run --release -p improvise-core --example
category_width_bench`, on an Apple M4 Max. Each category has 4 items and each
record carries 2 measures. The pivot is one category on rows, `_Measure` on
columns and the rest aggregated away. Figures are the best of 5 (the store
build is the best of 3), and they held across two runs.
| categories | records | build store | pivot, every cell | records build | records draw |
|---|---|---|---|---|---|
| 12 | 2,500 | 26–30 ms | 1.6 ms | 79 ms | 1.6 ms |
| 24 | 2,500 | 80–83 ms | 3.0–3.7 ms | 82 ms | 3.7 ms |
| 12 | 5,000 | 54–57 ms | 3.2–3.9 ms | 302 ms | 3.1 ms |
| 24 | 5,000 | 163–172 ms | 6.3–8.0 ms | 321 ms | 7.0 ms |
Doubling the width doubles the per-cell work and triples loading the store,
which also builds and hashes CellKeys twice as long. The records-mode build is
the outlier, and width is not its cause: it grows 4× when the **records**
double, at either width. `RecordRow::group` finds each cell's row with a linear
scan (improvise-sk32).
### Seating takes the kind, because a name cannot carry it (improvise-cj4u) ### 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 `View::on_category_added` takes a `CategoryKind` beside the name. It used to
+4 -3
View File
@@ -165,7 +165,7 @@ share, which is why the existing suite was silent — the same shape as the
`Category::kind` rather than the emitted text. `Category::kind` rather than the emitted text.
The consequence was a lost document, not a lost flag. `Label` sits outside 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 `MAX_CATEGORIES` on purpose (12 then, 24 since improvise-felo.1), and the import wizard's own precondition
scores a label as zero, so it will happily build twelve categories plus a 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 label. On load every label came back `Regular`, the thirteenth was refused, and
`parse_md` failed: **any document where regular + label exceeded twelve was `parse_md` failed: **any document where regular + label exceeded twelve was
@@ -643,8 +643,9 @@ Preconditions live in `validation_error()` and are checked **on the step where
the user can still fix them** — never deferred to `build_model` after the final the user can still fix them** — never deferred to `build_model` after the final
confirm. Two gates earn that: too many categories (improvise-mzv) and no confirm. Two gates earn that: too many categories (improvise-mzv) and no
category at all (improvise-gfq); both are fixable with `c` at `ReviewProposals`, category at all (improvise-gfq); both are fixable with `c` at `ReviewProposals`,
and both used to refuse only at the last keystroke. `MAX_CATEGORIES` here mirrors and both used to refuse only at the last keystroke. `MAX_CATEGORIES` here is the
the model's private limit and counts regular categories only. model's own constant, which counts regular categories only. Until improvise-felo.1
it was a private copy (core.md §9).
`NameModel` is terminal for `advance`: leaving it is a **build**, not a step `NameModel` is terminal for `advance`: leaving it is a **build**, not a step
change. `build()` parks the workbook in `built` on `Done`; a build failure leaves change. `build()` parks the workbook in `built` on `Done`; a build failure leaves
+3 -2
View File
@@ -69,7 +69,7 @@ same effect without the early return.
three shapes; dispatch pattern-matches exhaustively. three shapes; dispatch pattern-matches exhaustively.
- `CategoryKind` is `Regular | VirtualIndex | VirtualDim | VirtualMeasure | Label`. - `CategoryKind` is `Regular | VirtualIndex | VirtualDim | VirtualMeasure | Label`.
Business rules (e.g., the 12-category limit counts only `Regular`) are Business rules (e.g., the 24-category limit counts only `Regular`) are
enforced by matching on the enum, not by checking name prefixes. Virtual enforced by matching on the enum, not by checking name prefixes. Virtual
categories (`_Index`, `_Dim`, `_Measure`) always exist: `_Index` and `_Dim` categories (`_Index`, `_Dim`, `_Measure`) always exist: `_Index` and `_Dim`
support drill-down/records mode; `_Measure` holds numeric data fields and support drill-down/records mode; `_Measure` holds numeric data fields and
@@ -99,7 +99,8 @@ sort. Property tests verify this invariant.
- `CellKey::new()` is the only way to build a key — it always sorts. - `CellKey::new()` is the only way to build a key — it always sorts.
- `Category::add_item()` deduplicates by name and auto-assigns IDs via a private - `Category::add_item()` deduplicates by name and auto-assigns IDs via a private
counter. External code cannot fabricate an `ItemId`. counter. External code cannot fabricate an `ItemId`.
- `Model::add_category()` checks the 12-category limit before insertion. - `Model::add_category()` checks the category limit (`MAX_CATEGORIES`, 24)
before insertion.
- `Formula::new()` takes all required fields; there is no default/empty formula - `Formula::new()` takes all required fields; there is no default/empty formula
to accidentally leave half-initialized. to accidentally leave half-initialized.
+1 -1
View File
@@ -257,7 +257,7 @@ The second weekend (or the second half of the first weekend) is reserved for wha
The following are real issues that have been identified but are explicitly not launch-blocking. Do not start any of them as part of this plan. They can be addressed after launch based on what real users actually trip over. The following are real issues that have been identified but are explicitly not launch-blocking. Do not start any of them as part of this plan. They can be addressed after launch based on what real users actually trip over.
- **Do not** raise or restructure the `MAX_CATEGORIES = 12` limit, even though it's tight for some real use cases. - ~~**Do not** raise or restructure the `MAX_CATEGORIES = 12` limit, even though it's tight for some real use cases.~~ **Superseded** by the user's decision of 2026-09-25 ("12 categories is feeling small, we should increase to 24"). The limit is 24 (improvise-felo.1).
- **Do not** refactor the `ExecuteCommand` vim command match ladder, even though it duplicates registry logic. - **Do not** refactor the `ExecuteCommand` vim command match ladder, even though it duplicates registry logic.
- **Do not** fix the hardcoded `"Measure"` category name in `evaluate_aggregated`, even though it's the one place in the code that assumes a specific user-facing name. - **Do not** fix the hardcoded `"Measure"` category name in `evaluate_aggregated`, even though it's the one place in the code that assumes a specific user-facing name.
- **Do not** change `Model::evaluate` from a linear formula walk to a HashMap lookup, even though it's O(N·M·F) in the render hot path. - **Do not** change `Model::evaluate` from a linear formula walk to a HashMap lookup, even though it's O(N·M·F) in the render hot path.
+3 -3
View File
@@ -2479,8 +2479,8 @@ from `DocumentSnapshot` — it is no longer only the document.
**The fixture detail that matters**: a *duplicate* `AddCategory` is idempotent, so **The fixture detail that matters**: a *duplicate* `AddCategory` is idempotent, so
the obvious way to reach the refusal proves nothing. This fills to the obvious way to reach the refusal proves nothing. This fills to
`MAX_CATEGORIES` (12) and overflows. And the predecessor is an `AddItem` rather `MAX_CATEGORIES` and overflows. And the predecessor is an `AddItem` rather
than a `SetCell`, because adding eleven categories reshapes the view enough that than a `SetCell`, because filling to the limit reshapes the view enough that
a record-addressed predecessor is refused `RecordGone` **first** — hiding the arm a record-addressed predecessor is refused `RecordGone` **first** — hiding the arm
under test behind an earlier one. under test behind an earlier one.
@@ -3505,7 +3505,7 @@ handshake. The stronger form fails under both injections.
improvise-75ld. `App::apply_effects` bumped `layout_version` and raised improvise-75ld. `App::apply_effects` bumped `layout_version` and raised
`model_state.dirty` off the `EffectKind` **before** the `abort_effects` check, so `model_state.dirty` off the `EffectKind` **before** the `abort_effects` check, so
the refusing effect counted itself. Two arms, and the second is the reachable one: the refusing effect counted itself. Two arms, and the second is the reachable one:
a 13th category is refused (`MAX_CATEGORIES`) and aborts, while a *duplicate* a category past `MAX_CATEGORIES` is refused and aborts, while a *duplicate*
`add-cat Region` returns `Ok` and does not abort at all — a pure no-op that still `add-cat Region` returns `Ok` and does not abort at all — a pure no-op that still
refused every other client's next `Command::Model` with `Rejection::LayoutMoved` refused every other client's next `Command::Model` with `Rejection::LayoutMoved`
and made the browser worker rewrite the whole OPFS document (gotcha 41). and made the browser worker rewrite the whole OPFS document (gotcha 41).
+2 -2
View File
@@ -1002,7 +1002,7 @@ the error, rather than moving to a `Done` step with nothing to confirm.
### wizard_blocks_advance_when_over_category_limit ### wizard_blocks_advance_when_over_category_limit
Bug improvise-mzv: with more than `MAX_CATEGORIES` (12) accepted Category Bug improvise-mzv: with more than `MAX_CATEGORIES` (12 then, 24 since improvise-felo.1) accepted Category
proposals, the wizard let the user advance through every remaining step and proposals, the wizard let the user advance through every remaining step and
`build_model()` only failed at the final confirm. Advancing past `build_model()` only failed at the final confirm. Advancing past
`ReviewProposals` must be blocked, with a message telling the user to mark `ReviewProposals` must be blocked, with a message telling the user to mark
@@ -1352,7 +1352,7 @@ Faults injected, both against the fixed tree:
### a_document_at_the_category_limit_plus_a_label_reloads ### 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` 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 is outside `MAX_CATEGORIES` (12 then) by design — `Model::add_label_category` has no
limit check, and the import wizard's `proposed_category_count` scores a label 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 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 constructible and the wizard will build one from any CSV carrying a free-text
+3 -2
View File
@@ -3314,7 +3314,7 @@ mod tests {
/// Eleven regular categories: one short of the limit. /// Eleven regular categories: one short of the limit.
fn one_category_short_of_the_limit() -> App { fn one_category_short_of_the_limit() -> App {
let mut wb = Workbook::new("T"); let mut wb = Workbook::new("T");
for n in 0..11 { for n in 0..crate::model::MAX_CATEGORIES - 1 {
wb.add_category(format!("C{n}")).unwrap(); wb.add_category(format!("C{n}")).unwrap();
} }
App::new(wb, None) App::new(wb, None)
@@ -3464,12 +3464,13 @@ mod tests {
fn every_refusal_site_refuses_its_failure_and_only_its_failure() { fn every_refusal_site_refuses_its_failure_and_only_its_failure() {
type Setup = fn() -> App; type Setup = fn() -> App;
type Drive = fn(&mut App) -> bool; type Drive = fn(&mut App) -> bool;
let limit = format!("Maximum of {} categories", crate::model::MAX_CATEGORIES);
let cases: Vec<(&str, Setup, Drive, &str, Setup, Drive)> = vec![ let cases: Vec<(&str, Setup, Drive, &str, Setup, Drive)> = vec![
( (
"add-category past the limit", "add-category past the limit",
one_category_short_of_the_limit, one_category_short_of_the_limit,
|app| run_line(app, "add-category A . add-category B"), |app| run_line(app, "add-category A . add-category B"),
"Maximum of 12 categories", &limit,
one_category_short_of_the_limit, one_category_short_of_the_limit,
|app| run_line(app, "add-category A"), |app| run_line(app, "add-category A"),
), ),
+3 -3
View File
@@ -1912,8 +1912,8 @@ mod tests {
#[test] #[test]
fn add_category_past_the_limit_reports_the_failure() { fn add_category_past_the_limit_reports_the_failure() {
let mut app = test_app(); let mut app = test_app();
// test_app() holds 2 regular categories; fill up to the limit of 12. // test_app() holds 2 regular categories; fill up to the limit.
for i in 0..10 { for i in 0..crate::model::MAX_CATEGORIES - 2 {
AddCategory(format!("Cat{i}")).apply(&mut app); AddCategory(format!("Cat{i}")).apply(&mut app);
} }
app.view_state.status_msg = "Added category \"Overflow\"".to_string(); app.view_state.status_msg = "Added category \"Overflow\"".to_string();
@@ -1924,7 +1924,7 @@ mod tests {
.model .model
.category("Overflow") .category("Overflow")
.is_none(), .is_none(),
"the 13th category must not be added" "the category past the limit must not be added"
); );
assert!( assert!(
app.view_state.status_msg.contains("Maximum"), app.view_state.status_msg.contains("Maximum"),
+3 -2
View File
@@ -1177,7 +1177,7 @@ mod tests {
let mut host = host(); let mut host = host();
// Fill to MAX_CATEGORIES, so the frame's second command is refused by // Fill to MAX_CATEGORIES, so the frame's second command is refused by
// `Model::add_category` — a DUPLICATE add is idempotent and vacuous. // `Model::add_category` — a DUPLICATE add is idempotent and vacuous.
for n in 0..11 { for n in 0..crate::model::MAX_CATEGORIES - 1 {
host.app_mut() host.app_mut()
.model_state .model_state
.workbook .workbook
@@ -2979,7 +2979,8 @@ mod tests {
let fresh = || { let fresh = || {
let mut wb = workbook(); let mut wb = workbook();
wb.create_view("Scratch"); wb.create_view("Scratch");
(0..12).for_each(|n| drop(wb.add_category(format!("Filler{n}")))); (0..crate::model::MAX_CATEGORIES)
.for_each(|n| drop(wb.add_category(format!("Filler{n}"))));
let mut app = App::new(wb, None); let mut app = App::new(wb, None);
app.storage = Box::new(super::import_fixture_storage()); app.storage = Box::new(super::import_fixture_storage());
app.rebuild_layout(); app.rebuild_layout();
+1 -1
View File
@@ -3674,7 +3674,7 @@ mod tests {
item: "Gizmo".into(), item: "Gizmo".into(),
group: "Hardware".into(), group: "Hardware".into(),
}, },
// The 12-category limit; the fixture already holds two. // The category limit; the fixture already holds two.
ModelCommand::AddCategory("Overflow".into()), ModelCommand::AddCategory("Overflow".into()),
]; ];
@@ -0,0 +1,128 @@
//! What a wider document costs: the same records at 12 and at 24 regular
//! categories. improvise-felo.1; the argument is `context/code/core.md` §9.
use std::time::Instant;
use improvise_core::model::cell::{CellKey, CellValue};
use improvise_core::view::{Axis, GridLayout, ViewStep, records_steps};
use improvise_core::workbook::Workbook;
const MEASURE: &str = "_Measure";
const ITEMS: u64 = 4;
/// `records` records over `width` categories of `ITEMS` items, two measures
/// each; coordinates from a fixed LCG so both widths see comparable data.
fn document(width: usize, records: usize) -> Workbook {
let mut wb = Workbook::new("bench");
for c in 0..width {
wb.add_category(format!("C{c}")).expect("under the limit");
}
let mut state: u64 = 0x9E37_79B9_7F4A_7C15;
let mut next = move || {
state = state
.wrapping_mul(6_364_136_223_846_793_005)
.wrapping_add(1_442_695_040_888_963_407);
(state >> 33) % ITEMS
};
for _ in 0..records {
let coords: Vec<(String, String)> = (0..width)
.map(|c| (format!("C{c}"), format!("i{}", next())))
.collect();
for (name, value) in [("Cost", 3.0), ("Rev", 5.0)] {
let mut key = coords.clone();
key.push((MEASURE.to_string(), name.to_string()));
wb.model
.set_cell(CellKey::new(key), CellValue::Number(value));
}
}
for c in 0..width {
let cat = wb.model.category_mut(&format!("C{c}")).expect("added");
for i in 0..ITEMS {
cat.add_item(format!("i{i}"));
}
}
for name in ["Cost", "Rev"] {
wb.model
.category_mut(MEASURE)
.expect("virtual")
.add_item(name);
}
wb
}
/// Best of `runs`, in milliseconds.
fn best(runs: usize, mut f: impl FnMut()) -> f64 {
(0..runs)
.map(|_| {
let t = Instant::now();
f();
t.elapsed().as_secs_f64() * 1000.0
})
.fold(f64::INFINITY, f64::min)
}
fn pivot(wb: &mut Workbook, width: usize) -> f64 {
let view = wb.active_view_mut();
for c in 0..width {
view.set_axis(
&format!("C{c}"),
if c == 0 { Axis::Row } else { Axis::None },
);
}
view.set_axis(MEASURE, Axis::Column);
let (model, view) = (&wb.model, wb.active_view());
best(5, || {
let layout = GridLayout::with_frozen_records(model, view, None);
let eval = layout.evaluator(model);
for r in 0..layout.row_count() {
for c in 0..layout.col_count() {
let key = layout.cell_key(r, c).expect("in bounds");
std::hint::black_box(eval.cell(&key));
}
}
})
}
/// (build, draw): the records layout, then every cell's display text.
fn records(wb: &mut Workbook) -> (f64, f64) {
let steps = records_steps(&wb.model);
let view = wb.active_view_mut();
for step in steps {
if let ViewStep::Axis { category, axis } = step {
view.set_axis(&category, axis);
}
}
let (model, view) = (&wb.model, wb.active_view());
let build = best(5, || {
std::hint::black_box(GridLayout::with_frozen_records(model, view, None));
});
let layout = GridLayout::with_frozen_records(model, view, None);
let draw = best(5, || {
for r in 0..layout.row_count() {
for c in 0..layout.col_count() {
std::hint::black_box(layout.records_display(r, c));
}
}
});
(build, draw)
}
fn main() {
println!(
"{:>6} {:>8} {:>7} {:>9} {:>9} {:>14} {:>13}",
"width", "records", "cells", "store ms", "pivot ms", "rec build ms", "rec draw ms"
);
for records_n in [2_500, 5_000] {
for width in [12, 24] {
let mut built = None;
let store = best(3, || built = Some(document(width, records_n)));
let mut wb = built.expect("ran");
let cells = wb.model.data.iter_cells().count();
let p = pivot(&mut wb, width);
let (rb, rd) = records(&mut wb);
println!(
"{width:>6} {records_n:>8} {cells:>7} {store:>9.1} {p:>9.1} {rb:>14.1} {rd:>13.1}"
);
}
}
}
+1 -1
View File
@@ -4,4 +4,4 @@ pub mod record;
pub mod symbol; pub mod symbol;
pub mod types; pub mod types;
pub use types::{Aggregation, Model}; pub use types::{Aggregation, MAX_CATEGORIES, Model};
+18 -8
View File
@@ -7,7 +7,9 @@ use super::category::{Category, CategoryId, CategoryKind, MEASURE_CATEGORY};
use super::cell::{CellKey, CellValue, DataStore}; use super::cell::{CellKey, CellValue, DataStore};
use crate::formula::{AggFunc, Expr, Filter, Formula, FormulaId}; use crate::formula::{AggFunc, Expr, Filter, Formula, FormulaId};
const MAX_CATEGORIES: usize = 12; /// Regular categories a model may hold; labels and virtuals do not count.
/// Why 24 and what it costs: context/code/core.md §9 (improvise-felo.1).
pub const MAX_CATEGORIES: usize = 24;
/// Shared by `=`/`!=` **and** the division-by-zero guard, in both evaluators. /// Shared by `=`/`!=` **and** the division-by-zero guard, in both evaluators.
/// They must agree; why, and why 1e-10: context/code/core.md §4. /// They must agree; why, and why 1e-10: context/code/core.md §4.
@@ -1386,7 +1388,8 @@ mod model_tests {
assert_eq!(id, m.categories["C0"].id); assert_eq!(id, m.categories["C0"].id);
assert_eq!(m.categories.len(), before, "nothing was created"); assert_eq!(m.categories.len(), before, "nothing was created");
// The limit still refuses a category that WOULD be created. // The limit still refuses a category that WOULD be created.
assert!(m.add_category("C12").is_err(), "the limit still holds"); let fresh = format!("C{}", super::MAX_CATEGORIES);
assert!(m.add_category(fresh).is_err(), "the limit still holds");
} }
/// improvise-s08t — see context/test-evidence/core.md. /// improvise-s08t — see context/test-evidence/core.md.
@@ -1550,13 +1553,20 @@ mod model_tests {
assert_eq!(m.category_names().len(), 4); assert_eq!(m.category_names().len(), 4);
} }
/// The user's number (improvise-felo.1), spelled once here; every other
/// fill-to-the-cap test derives it from `MAX_CATEGORIES`.
#[test] #[test]
fn add_category_max_limit() { fn add_category_max_limit() {
let mut m = Model::new("Test"); let mut m = Model::new("Test");
for i in 0..12 { for i in 0..24 {
m.add_category(format!("Cat{i}")).unwrap(); m.add_category(format!("Cat{i}"))
.unwrap_or_else(|e| panic!("category {} of 24 refused: {e}", i + 1));
} }
assert!(m.add_category("TooMany").is_err()); let err = m.add_category("TooMany").expect_err("the 25th is refused");
assert!(
err.to_string().contains("24"),
"the refusal names the real limit: {err}"
);
} }
#[test] #[test]
@@ -3152,11 +3162,11 @@ mod five_category {
// 4 regular + 3 virtual. // 4 regular + 3 virtual.
assert_eq!(m.category_names().len(), 7); assert_eq!(m.category_names().len(), 7);
let mut m2 = build_model(); let mut m2 = build_model();
for i in 0..8 { for i in 0..super::MAX_CATEGORIES - 4 {
m2.add_category(format!("Extra{i}")).unwrap(); m2.add_category(format!("Extra{i}")).unwrap();
} }
// 12 regular + 3 virtuals = 15. // The limit's worth of regular categories, plus 3 virtuals.
assert_eq!(m2.category_names().len(), 15); assert_eq!(m2.category_names().len(), super::MAX_CATEGORIES + 3);
assert!(m2.add_category("OneMore").is_err()); assert!(m2.add_category("OneMore").is_err());
} }
} }
+11 -9
View File
@@ -8,13 +8,10 @@ use super::analyzer::{
extract_date_component, find_array_paths, extract_date_component, find_array_paths,
}; };
use crate::formula::parse_formula; use crate::formula::parse_formula;
use crate::model::MAX_CATEGORIES;
use crate::model::cell::{CellKey, CellValue}; use crate::model::cell::{CellKey, CellValue};
use crate::workbook::Workbook; use crate::workbook::Workbook;
/// Mirrors the model's private `MAX_CATEGORIES` (regular categories only), to
/// pre-check proposals before the wizard's confirm step (improvise-mzv).
const MAX_CATEGORIES: usize = 12;
// ── Pipeline (no UI state) ──────────────────────────────────────────────────── // ── Pipeline (no UI state) ────────────────────────────────────────────────────
/// Pure data + logic for turning a JSON value into a Model. /// Pure data + logic for turning a JSON value into a Model.
@@ -1379,7 +1376,8 @@ mod tests {
/// the final confirm. Evidence: `context/test-evidence/formula-and-io.md`. /// the final confirm. Evidence: `context/test-evidence/formula-and-io.md`.
#[test] #[test]
fn wizard_blocks_advance_when_over_category_limit() { fn wizard_blocks_advance_when_over_category_limit() {
let mut w = ImportWizard::new(category_records(13)); let over = crate::model::MAX_CATEGORIES + 1;
let mut w = ImportWizard::new(category_records(over));
assert_eq!(w.step, WizardStep::ReviewProposals); assert_eq!(w.step, WizardStep::ReviewProposals);
assert_eq!( assert_eq!(
w.pipeline w.pipeline
@@ -1387,8 +1385,8 @@ mod tests {
.iter() .iter()
.filter(|p| p.accepted && p.kind == FieldKind::Category) .filter(|p| p.accepted && p.kind == FieldKind::Category)
.count(), .count(),
13, over,
"sanity: all 13 fields proposed as accepted categories" "sanity: every field proposed as an accepted category"
); );
w.advance(); w.advance();
@@ -1405,11 +1403,15 @@ mod tests {
msg.contains("Measure"), msg.contains("Measure"),
"message should tell the user how to fix it: {msg}" "message should tell the user how to fix it: {msg}"
); );
assert!(
msg.contains(&format!("max {}", crate::model::MAX_CATEGORIES)),
"message should name the model's limit: {msg}"
);
} }
#[test] #[test]
fn wizard_allows_advance_at_exactly_category_limit() { fn wizard_allows_advance_at_exactly_category_limit() {
let mut w = ImportWizard::new(category_records(12)); let mut w = ImportWizard::new(category_records(crate::model::MAX_CATEGORIES));
assert_eq!(w.step, WizardStep::ReviewProposals); assert_eq!(w.step, WizardStep::ReviewProposals);
w.advance(); w.advance();
assert_ne!(w.step, WizardStep::ReviewProposals); assert_ne!(w.step, WizardStep::ReviewProposals);
@@ -1417,7 +1419,7 @@ mod tests {
#[test] #[test]
fn wizard_advance_unblocks_after_marking_field_as_measure() { fn wizard_advance_unblocks_after_marking_field_as_measure() {
let mut w = ImportWizard::new(category_records(13)); let mut w = ImportWizard::new(category_records(crate::model::MAX_CATEGORIES + 1));
w.advance(); w.advance();
assert_eq!(w.step, WizardStep::ReviewProposals); assert_eq!(w.step, WizardStep::ReviewProposals);
@@ -1187,7 +1187,7 @@ fn a_label_category_survives_the_round_trip_as_a_label() {
#[test] #[test]
fn a_document_at_the_category_limit_plus_a_label_reloads() { fn a_document_at_the_category_limit_plus_a_label_reloads() {
let mut wb = Workbook::new("T"); let mut wb = Workbook::new("T");
for i in 0..12 { for i in 0..crate::model::MAX_CATEGORIES {
wb.add_category(format!("C{i}")).unwrap(); wb.add_category(format!("C{i}")).unwrap();
} }
wb.add_label_category("Note").unwrap(); wb.add_label_category("Note").unwrap();
@@ -1197,7 +1197,10 @@ fn a_document_at_the_category_limit_plus_a_label_reloads() {
loaded.model.category("Note").unwrap().kind, loaded.model.category("Note").unwrap().kind,
CategoryKind::Label CategoryKind::Label
); );
assert_eq!(loaded.model.regular_category_names().len(), 12); assert_eq!(
loaded.model.regular_category_names().len(),
crate::model::MAX_CATEGORIES
);
} }
/// A file written before the `kind:` line existed reads its categories as /// A file written before the `kind:` line existed reads its categories as