From 227ba54667105254a7c2bea2e47244062fdb7a1f Mon Sep 17 00:00:00 2001 From: "Claude (adr007-slug)" Date: Fri, 9 Oct 2026 12:43:28 -0700 Subject: [PATCH 01/11] feat(core): slugify, id syntax, natural order and display suffixes per ADR 007 amendment 1 (improvise-cq1q.14) Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- crates/improvise-core/src/model/id.rs | 340 ++++++++++++++++++++++---- 1 file changed, 297 insertions(+), 43 deletions(-) diff --git a/crates/improvise-core/src/model/id.rs b/crates/improvise-core/src/model/id.rs index 7448a647..5340cc09 100644 --- a/crates/improvise-core/src/model/id.rs +++ b/crates/improvise-core/src/model/id.rs @@ -1,6 +1,7 @@ -//! How an id is made: `slugify`, and the document's `IdAllocator` (ADR 007 clause 4). -//! Reasoning: `context/code/core.md` §20. -use std::collections::HashSet; +//! How an id is made: `slugify`, and the document's `IdAllocator` (ADR 007 clause 4 +//! and its amendment). Reasoning: `context/code/core.md` §20. +use std::cmp::Ordering; +use std::collections::{HashMap, HashSet}; /// What a generated id names; spells the `` of an empty slug's `-`. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -9,6 +10,7 @@ pub enum IdKind { Item, Formula, View, + Group, } impl IdKind { @@ -18,38 +20,111 @@ impl IdKind { IdKind::Item => "item", IdKind::Formula => "formula", IdKind::View => "view", + IdKind::Group => "group", } } } -/// The only ids that may begin with `_` (answer 23). -pub const VIRTUAL_IDS: [&str; 3] = ["_index", "_dim", "_measure"]; +/// The only ids that may begin with `_` (amendment 1, clause 1). +pub const VIRTUAL_IDS: [&str; 5] = ["_index", "_dim", "_measure", "_records", "_drill"]; -/// A display name's slug; empty when nothing survives (answers 16, 18, 21, 23, 26). +/// A display name's slug; empty when nothing survives (amendment 1, clause 4). pub fn slugify(display: &str) -> String { let folded = unicase::UniCase::new(display).to_folded_case(); - let kept: String = underscore_space_runs(&folded) + let mut slug = String::new(); + for c in folded .chars() - .filter(|&c| in_id_alphabet(c)) - .collect(); - kept.trim_start_matches('_').to_string() + .map(|c| if in_slug_alphabet(c) { c } else { '_' }) + { + if !(c == '_' && slug.ends_with('_')) { + slug.push(c); + } + } + let trimmed = slug.trim_matches(|c| c == '_' || c == '-'); + if trimmed.starts_with(|c: char| c.is_ascii_digit()) { + format!("n{trimmed}") + } else { + trimmed.to_string() + } } -fn underscore_space_runs(s: &str) -> String { - let (out, _) = s - .chars() - .fold((String::new(), false), |(mut out, prev), c| { - let space = c == ' '; - if !(space && prev) { - out.push(if space { '_' } else { c }); - } - (out, space) - }); - out +fn in_slug_alphabet(c: char) -> bool { + matches!(c, 'a'..='z' | '0'..='9' | '-') } -fn in_id_alphabet(c: char) -> bool { - matches!(c, 'a'..='z' | '0'..='9' | '-' | '_') +/// Whether `id` has the syntax of a generated id: a letter, then `a`-`z`, `0`-`9`, +/// `-` and single underscores, never ending in `_` or `-`. A virtual id fails it. +pub fn is_id_syntax(id: &str) -> bool { + let b = id.as_bytes(); + let edges = matches!(b.first(), Some(b'a'..=b'z')) && !matches!(b.last(), Some(b'_' | b'-')); + edges && b.windows(2).all(|w| w != b"__") && id.chars().all(|c| in_slug_alphabet(c) || c == '_') +} + +/// Ids in natural order: digit runs by value, then fewer leading zeros first. +pub fn natural_cmp(a: &str, b: &str) -> Ordering { + let (mut ra, mut rb) = (runs(a), runs(b)); + loop { + match (ra.next(), rb.next()) { + (None, None) => return Ordering::Equal, + (None, Some(_)) => return Ordering::Less, + (Some(_), None) => return Ordering::Greater, + (Some(x), Some(y)) => match run_cmp(x, y) { + Ordering::Equal => {} + other => return other, + }, + } + } +} + +fn runs(s: &str) -> impl Iterator { + let mut rest = s; + std::iter::from_fn(move || { + let digit = rest.chars().next()?.is_ascii_digit(); + let end = rest + .find(|c: char| c.is_ascii_digit() != digit) + .unwrap_or(rest.len()); + let (run, tail) = rest.split_at(end); + rest = tail; + Some(run) + }) +} + +fn run_cmp(x: &str, y: &str) -> Ordering { + let digits = |r: &str| r.starts_with(|c: char| c.is_ascii_digit()); + if digits(x) && digits(y) { + let (vx, vy) = (x.trim_start_matches('0'), y.trim_start_matches('0')); + vx.len() + .cmp(&vy.len()) + .then_with(|| vx.cmp(vy)) + .then_with(|| x.len().cmp(&y.len())) + } else { + x.cmp(y) + } +} + +/// What each `(id, display)` pair is shown as: equal displays get ` #2`, ` #3`, … +/// in natural id order, skipping numbers a literal display already holds. +pub fn display_suffixes(items: &[(&str, &str)]) -> Vec { + let literal: HashSet<&str> = items.iter().map(|&(_, d)| d).collect(); + let mut groups: HashMap<&str, Vec> = HashMap::new(); + for (i, &(_, d)) in items.iter().enumerate() { + groups.entry(d).or_default().push(i); + } + let mut shown: Vec = items.iter().map(|&(_, d)| d.to_string()).collect(); + for (display, mut at) in groups { + at.sort_by(|&i, &j| natural_cmp(items[i].0, items[j].0)); + let mut n = 2; + for &i in at.iter().skip(1) { + shown[i] = loop { + let candidate = format!("{display} #{n}"); + n += 1; + if !literal.contains(candidate.as_str()) { + break candidate; + } + }; + } + } + shown } /// Why a hand-written id cannot enter the document. @@ -81,12 +156,13 @@ impl IdAllocator { Self::default() } - /// Admit a hand-written id verbatim: it is canonical and never renamed (answer 29). + /// Admit a hand-written id verbatim: it is canonical and never renamed. pub fn claim(&mut self, id: &str) -> Result<(), ClaimError> { - let refusal = if id.is_empty() || !id.chars().all(in_id_alphabet) { - Some(ClaimError::Malformed(id.to_string())) - } else if id.starts_with('_') && !VIRTUAL_IDS.contains(&id) { + let virtual_id = VIRTUAL_IDS.contains(&id); + let refusal = if id.starts_with('_') && !virtual_id { Some(ClaimError::Reserved(id.to_string())) + } else if !virtual_id && !is_id_syntax(id) { + Some(ClaimError::Malformed(id.to_string())) } else if self.seen.contains(id) { Some(ClaimError::Duplicate(id.to_string())) } else { @@ -132,16 +208,26 @@ mod tests { use proptest::prelude::*; #[test] - fn slugify_follows_the_adrs_examples() { + fn slugify_follows_the_amendments_table() { let cases = [ - ("East", "east"), ("Time Period", "time_period"), - ("Coastal_East", "coastal_east"), - ("Q1-2024", "q1-2024"), - ("Region(code)", "regioncode"), - ("Café", "caf"), - ("_Draft", "draft"), + ("Time\tPeriod", "time_period"), + ("Time \u{200B} Period", "time_period"), + ("a_ b", "a_b"), + ("a__b", "a_b"), + ("Q1 - 2024", "q1_-_2024"), + ("East ", "east"), + ("East_", "east"), + ("-East-", "east"), + ("Region(code)", "region_code"), + ("Crème Brûlée", "cr_me_br_l_e"), + ("2024", "n2024"), + ("_2024", "n2024"), + ("2024-Q1", "n2024-q1"), ("Ü", ""), + ("-", ""), + ("%", ""), + ("", ""), ]; for (display, slug) in cases { assert_eq!(slugify(display), slug, "slugify({display:?})"); @@ -156,15 +242,21 @@ mod tests { } #[test] - fn slugify_collapses_a_run_of_spaces_to_one_underscore() { - assert_eq!(slugify("Time Period"), "time_period"); - assert_eq!(slugify("East "), "east_"); + fn slugify_trims_hyphens_and_underscores_until_neither_is_at_an_end() { + assert_eq!(slugify("_-_East-_-"), "east"); + assert_eq!(slugify("-_-"), ""); } #[test] - fn slugify_keeps_an_underscore_the_display_already_had() { - assert_eq!(slugify("a_ b"), "a__b"); - assert_eq!(slugify("a__b"), "a__b"); + fn slugify_trims_before_it_prepends_the_n() { + assert_eq!(slugify("-_7up"), "n7up"); + assert_eq!(slugify("n-7"), "n-7"); + } + + #[test] + fn slugify_keeps_a_hyphen_between_underscores() { + assert_eq!(slugify("a - b"), "a_-_b"); + assert_eq!(slugify("a_-b"), "a_-b"); } #[test] @@ -173,6 +265,8 @@ mod tests { assert_eq!(ids.generate(IdKind::Category, "Ü"), "category-1"); assert_eq!(ids.generate(IdKind::Item, "Ö"), "item-2"); assert_eq!(ids.generate(IdKind::View, ""), "view-3"); + assert_eq!(ids.generate(IdKind::Group, "-"), "group-4"); + assert_eq!(ids.generate(IdKind::Formula, "%"), "formula-5"); } #[test] @@ -197,6 +291,15 @@ mod tests { assert_eq!(ids.generate(IdKind::Item, "EAST"), "east-3"); } + #[test] + fn a_hyphenated_slug_and_a_counter_suffix_never_collide() { + let mut ids = IdAllocator::new(); + assert_eq!(ids.generate(IdKind::Item, "East"), "east"); + assert_eq!(ids.generate(IdKind::Item, "East-1"), "east-1"); + assert_eq!(ids.generate(IdKind::Item, "east"), "east-2"); + assert_eq!(ids.generate(IdKind::Item, "East-1"), "east-1-3"); + } + #[test] fn a_claimed_id_keeps_its_spelling_and_generation_steps_past_it() { let mut ids = IdAllocator::new(); @@ -224,10 +327,21 @@ mod tests { ); } + #[test] + fn claim_refuses_what_a_generated_id_could_not_be() { + let mut ids = IdAllocator::new(); + for bad in ["-x", "x-", "2024", "a__b", "x_", "-", "a b", "é"] { + assert_eq!(ids.claim(bad), Err(ClaimError::Malformed(bad.into()))); + } + for good in ["east-1", "item-4", "a-7", "q1_-_2024", "a_b", "x"] { + assert_eq!(ids.claim(good), Ok(()), "{good}"); + } + } + #[test] fn claim_refuses_an_id_that_only_begins_like_a_virtual_one() { let mut ids = IdAllocator::new(); - for lookalike in ["_indexer", "_measure-1", "_dim_2", "_measures"] { + for lookalike in ["_indexer", "_measure-1", "_dim_2", "_measures", "_recordsx"] { assert_eq!( ids.claim(lookalike), Err(ClaimError::Reserved(lookalike.into())) @@ -235,10 +349,20 @@ mod tests { } } + #[test] + fn claim_admits_each_virtual_id_once() { + let mut ids = IdAllocator::new(); + for v in VIRTUAL_IDS { + assert_eq!(ids.claim(v), Ok(())); + assert_eq!(ids.claim(v), Err(ClaimError::Duplicate(v.into()))); + } + } + #[test] fn a_display_with_leading_spaces_never_slugs_into_the_reserved_space() { assert_eq!(slugify(" Draft"), "draft"); assert_eq!(slugify(" _ Draft"), "draft"); + assert_eq!(slugify("_Drill"), "drill"); } #[test] @@ -272,7 +396,138 @@ mod tests { assert_eq!(run(), run()); } + #[test] + fn natural_order_puts_a_counter_suffix_in_numeric_order() { + let mut ids = vec!["profit-10", "profit-2", "profit", "profit-1"]; + ids.sort_by(|a, b| natural_cmp(a, b)); + assert_eq!(ids, ["profit", "profit-1", "profit-2", "profit-10"]); + } + + #[test] + fn natural_order_ties_on_value_go_to_fewer_leading_zeros() { + assert_eq!(natural_cmp("a-7", "a-007"), Ordering::Less); + assert_eq!(natural_cmp("a-007", "a-7"), Ordering::Greater); + assert_eq!(natural_cmp("a-07", "a-007"), Ordering::Less); + assert_eq!(natural_cmp("a-7", "a-7"), Ordering::Equal); + } + + #[test] + fn natural_order_compares_digit_runs_of_any_length_by_value() { + let big = "9".repeat(40); + let bigger = format!("1{}", "0".repeat(40)); + assert_eq!( + natural_cmp(&format!("x{big}"), &format!("x{bigger}")), + Ordering::Less + ); + assert_eq!(natural_cmp("x9", "x10"), Ordering::Less); + assert_eq!(natural_cmp("x000", "x0"), Ordering::Greater); + } + + #[test] + fn natural_order_puts_the_id_that_runs_out_of_runs_first() { + assert_eq!(natural_cmp("a", "a-1"), Ordering::Less); + assert_eq!(natural_cmp("a-1", "a-1-b"), Ordering::Less); + assert_eq!(natural_cmp("a-1b", "a-1"), Ordering::Greater); + } + + #[test] + fn natural_order_compares_non_digit_runs_by_bytes() { + assert_eq!(natural_cmp("b1", "a2"), Ordering::Greater); + assert_eq!(natural_cmp("a-1", "a_1"), Ordering::Less); + } + + #[test] + fn a_suffix_numbers_equal_displays_in_natural_id_order() { + let items = [ + ("profit-10", "Profit"), + ("profit", "Profit"), + ("profit-2", "Profit"), + ("profit-1", "Profit"), + ("cost", "Cost"), + ]; + assert_eq!( + display_suffixes(&items), + [ + "Profit #4", + "Profit", + "Profit #3", + "Profit #2", + "Cost" + ] + ); + } + + #[test] + fn a_suffix_skips_a_number_a_literal_display_already_holds() { + let items = [ + ("profit", "Profit"), + ("profit-1", "Profit"), + ("profit-2", "Profit"), + ("p2", "Profit #2"), + ]; + assert_eq!( + display_suffixes(&items), + ["Profit", "Profit #3", "Profit #4", "Profit #2"] + ); + } + + #[test] + fn a_suffix_is_gapless_when_a_thing_goes_away() { + let three = [("p", "Profit"), ("p-1", "Profit"), ("p-2", "Profit")]; + assert_eq!(display_suffixes(&three), ["Profit", "Profit #2", "Profit #3"]); + let two = [("p", "Profit"), ("p-2", "Profit")]; + assert_eq!(display_suffixes(&two), ["Profit", "Profit #2"]); + } + + #[test] + fn a_unique_display_is_shown_unchanged() { + let items = [("a", "East"), ("b", "West")]; + assert_eq!(display_suffixes(&items), ["East", "West"]); + assert!(display_suffixes(&[]).is_empty()); + } + + fn id_text() -> impl Strategy { + "[a-c0-3_-]{0,7}" + } + proptest! { + #[test] + fn slugify_output_has_the_id_syntax_and_is_a_fixed_point(display in ".{0,12}") { + let slug = slugify(&display); + prop_assert!(slug.is_empty() || is_id_syntax(&slug), "{slug:?}"); + prop_assert_eq!(slugify(&slug), slug); + } + + #[test] + fn id_syntax_agrees_with_its_regular_expression(id in "[a-c0-9_-]{0,8}") { + let spelled = id.starts_with(|c: char| c.is_ascii_lowercase()) + && !id.ends_with(['_', '-']) + && !id.contains("__"); + prop_assert_eq!(is_id_syntax(&id), spelled); + } + + #[test] + fn natural_order_is_a_total_order(a in id_text(), b in id_text(), c in id_text()) { + prop_assert_eq!(natural_cmp(&a, &b), natural_cmp(&b, &a).reverse()); + prop_assert_eq!(natural_cmp(&a, &b) == Ordering::Equal, a == b); + if natural_cmp(&a, &b) != Ordering::Greater && natural_cmp(&b, &c) != Ordering::Greater { + prop_assert!(natural_cmp(&a, &c) != Ordering::Greater); + } + } + + #[test] + fn suffixed_displays_are_distinct_when_the_literals_are_not_already_clashing( + ids in proptest::collection::hash_set("[a-c][0-9]{0,2}", 0..8), + display in "[AB]( #[2-4])?" + ) { + let mut ids: Vec = ids.into_iter().collect(); + ids.sort_by(|a, b| natural_cmp(a, b)); + let items: Vec<(&str, &str)> = ids.iter().map(|i| (i.as_str(), display.as_str())).collect(); + let shown = display_suffixes(&items); + let distinct: HashSet<&String> = shown.iter().collect(); + prop_assert_eq!(distinct.len(), shown.len()); + } + #[test] fn generated_ids_are_distinct_well_formed_and_never_reserved( displays in proptest::collection::vec(".{0,6}", 0..24) @@ -281,8 +536,7 @@ mod tests { let mut out = HashSet::new(); for d in &displays { let id = ids.generate(IdKind::Item, d); - prop_assert!(!id.is_empty() && !id.starts_with('_'), "{id:?}"); - prop_assert!(id.chars().all(|c| matches!(c, 'a'..='z' | '0'..='9' | '-' | '_'))); + prop_assert!(is_id_syntax(&id), "{id:?}"); prop_assert!(out.insert(id.clone()), "reissued {id:?}"); } } From e0bd0be3d5946188a56b93891bb718187e05f099 Mon Sep 17 00:00:00 2001 From: "Claude (adr007-2c)" Date: Fri, 9 Oct 2026 12:48:50 -0700 Subject: [PATCH 02/11] feat(display): status messages and the view title show displays, keys keep addressing (improvise-cq1q.13) Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- .../improvise-command/src/command/cmd/cell.rs | 18 ++ .../improvise-command/src/command/cmd/core.rs | 16 +- .../improvise-command/src/command/cmd/mode.rs | 4 +- .../src/command/cmd/panel.rs | 76 +++++++- .../improvise-command/src/command/cmd/tile.rs | 27 ++- crates/improvise-command/src/ui/app.rs | 31 ++- crates/improvise-protocol/src/lib.rs | 9 +- crates/improvise-protocol/src/render.rs | 26 +++ crates/improvise-protocol/src/resolve.rs | 183 ++++++++++++++++-- crates/improvise-protocol/src/viewmodel.rs | 18 +- crates/improvise-wasm-client/src/render.rs | 20 +- src/draw.rs | 26 ++- src/ui/grid.rs | 9 +- 13 files changed, 417 insertions(+), 46 deletions(-) diff --git a/crates/improvise-command/src/command/cmd/cell.rs b/crates/improvise-command/src/command/cmd/cell.rs index 7bcb3112..d3b43150 100644 --- a/crates/improvise-command/src/command/cmd/cell.rs +++ b/crates/improvise-command/src/command/cmd/cell.rs @@ -494,6 +494,24 @@ mod tests { app } + #[test] + fn a_refusal_names_the_formulas_display_not_its_key() { + let mut app = formula_app(); + let measure = app.model_state.workbook.model.category_mut("_Measure"); + let measure = measure.unwrap(); + measure.add_item("Profit"); + measure.set_item_display("Profit", "Operating profit after all costs"); + app.cursor_mut().selected = (1, 2); + press(&mut app, 'p'); + assert!( + app.view_state + .status_msg + .contains("Operating profit after all costs is computed"), + "got {:?}", + app.view_state.status_msg + ); + } + /// improvise-54g1 — see context/test-evidence/command.md. #[test] fn p_onto_a_formula_owned_cell_writes_nothing_and_names_the_formula() { diff --git a/crates/improvise-command/src/command/cmd/core.rs b/crates/improvise-command/src/command/cmd/core.rs index 876e6b59..6491efc5 100644 --- a/crates/improvise-command/src/command/cmd/core.rs +++ b/crates/improvise-command/src/command/cmd/core.rs @@ -23,7 +23,8 @@ pub const INSIDE_A_DRILL_REFUSAL: &str = "A value is not staged inside a drill — edit the cell instead"; /// What a command says when the cell it would write is a formula's result. -pub fn derived_refusal(target: &str) -> String { +pub fn derived_refusal(model: &Model, target: &str) -> String { + let target = model.item_display("_Measure", target); format!("{target} is computed by a formula — edit the formula, not the cell") } @@ -38,8 +39,10 @@ pub fn uneditable(model: &Model, layout: &GridLayout, row: usize, col: usize) -> } .to_string(), ), - Some(CellId::Derived { target }) => Some(derived_refusal(&target)), - Some(CellId::NonItem { category, item }) => Some(non_item_refusal(&category, &item)), + Some(CellId::Derived { target }) => Some(derived_refusal(model, &target)), + Some(CellId::NonItem { category, item }) => { + Some(non_item_refusal(model.category_display(&category), &item)) + } Some(CellId::Record(_) | CellId::Create(_) | CellId::Coordinate { .. }) | None => None, } } @@ -129,11 +132,14 @@ impl<'a> CmdContext<'a> { match self.cursor_cell_id() { Some(CellId::Record(_)) => self.stored_cell_key(), Some(CellId::Aggregate(_)) => Err(self.aggregate_refusal()), - Some(CellId::Derived { target }) => Err(derived_refusal(&target)), + Some(CellId::Derived { target }) => Err(derived_refusal(self.model, &target)), Some(CellId::Create(_) | CellId::Coordinate { .. }) => { Err(NO_VALUE_REFUSAL.to_string()) } - Some(CellId::NonItem { category, item }) => Err(non_item_refusal(&category, &item)), + Some(CellId::NonItem { category, item }) => Err(non_item_refusal( + self.model.category_display(&category), + &item, + )), None => Err(match self.cell_key() { Some(_) => NO_VALUE_REFUSAL, None => NO_CELL, diff --git a/crates/improvise-command/src/command/cmd/mode.rs b/crates/improvise-command/src/command/cmd/mode.rs index dfdf28a2..9db0b1e2 100644 --- a/crates/improvise-command/src/command/cmd/mode.rs +++ b/crates/improvise-command/src/command/cmd/mode.rs @@ -386,6 +386,7 @@ impl Cmd for EditOrDrill { // the synthetic key names no measure, so the record's key is resolved. if let Some(formula) = ctx.owning_formula() { return vec![effect::refuse(super::core::derived_refusal( + ctx.model, &formula.target, ))]; } @@ -393,7 +394,8 @@ impl Cmd for EditOrDrill { // refused later, under a minibuffer that hides why (improvise-yc9r). if let Some(improvise_protocol::CellId::NonItem { category, item }) = ctx.cursor_cell_id() { return vec![effect::refuse(improvise_protocol::non_item_refusal( - &category, &item, + ctx.model.category_display(&category), + &item, ))]; } vec![ diff --git a/crates/improvise-command/src/command/cmd/panel.rs b/crates/improvise-command/src/command/cmd/panel.rs index 14f08c97..83855380 100644 --- a/crates/improvise-command/src/command/cmd/panel.rs +++ b/crates/improvise-command/src/command/cmd/panel.rs @@ -1,4 +1,6 @@ -use improvise_protocol::viewmodel::axis_change_status; +use improvise_protocol::viewmodel::{ + axis_change_status, deleted_category_status, deleted_item_status, filter_status, +}; use crate::ui::app::AppMode; use crate::ui::effect::{self, Effect, Panel, ViewTarget}; @@ -186,6 +188,53 @@ mod tests { ); } + /// improvise-cq1q.13: evidence in `context/test-evidence/core.md`. + #[test] + fn a_panel_status_names_what_a_person_sees_not_the_keys() { + const TYPE: &str = "Kind of food and drink"; + const FOOD: &str = "Foodstuffs of every sort"; + let mut wb = two_cat_model(); + let ty = wb.model.category_mut("Type").unwrap(); + ty.set_display(TYPE); + ty.set_item_display("Food", FOOD); + let layout = make_layout(&wb); + let reg = make_registry(); + let mut ctx = make_ctx(&wb, &layout, ®); + let expanded = std::collections::HashSet::from(["Type".to_string()]); + ctx.expanded_cats = &expanded; + let said = |effects: &[Box]| format!("{:?}", effects.last().unwrap()); + + ctx.cat_panel_cursor = 3; + assert_eq!(ctx.cat_at_cursor().as_deref(), Some("Type"), "setup"); + assert!(said(&CycleAxisAtCursor.execute(&ctx)).contains(&format!("{TYPE} → "))); + assert!(said(&DeleteCategoryAtCursor.execute(&ctx)).contains(&format!("'{TYPE}'"))); + let prompt = format!("{:?}", OpenItemAddAtCursor.execute(&ctx)); + assert!( + prompt.contains(&format!("add item to {TYPE}: ")), + "{prompt}" + ); + assert!( + prompt.contains("category: \"Type\""), + "the key stays: {prompt}" + ); + + ctx.cat_panel_cursor = 4; + let filter = said(&FilterToItem.execute(&ctx)); + assert!( + filter.contains(&format!("Filter: {TYPE} = {FOOD}")), + "{filter}" + ); + let delete = format!("{:?}", DeleteCategoryAtCursor.execute(&ctx)); + assert!( + delete.contains(&format!("Deleted item '{FOOD}' from '{TYPE}'")), + "{delete}" + ); + assert!( + delete.contains("item: \"Food\""), + "the effect still names the key: {delete}" + ); + } + #[test] fn cycle_panel_focus_with_no_panels_open() { let m = two_cat_model(); @@ -718,8 +767,12 @@ impl Cmd for CycleAxisAtCursor { ViewTarget::Active, cat_name.clone(), ))]; - effects - .extend(cycled.map(|axis| effect::set_status(axis_change_status(&cat_name, axis)))); + effects.extend(cycled.map(|axis| { + effect::set_status(axis_change_status( + ctx.model.category_display(&cat_name), + axis, + )) + })); effects } else { vec![effect::refuse( @@ -738,7 +791,10 @@ impl Cmd for OpenItemAddAtCursor { } fn execute(&self, ctx: &CmdContext) -> Vec> { if let Some(cat_name) = ctx.cat_at_cursor() { - vec![effect::change_mode(AppMode::item_add(cat_name))] + let shown = ctx.model.category_display(&cat_name).to_string(); + vec![effect::change_mode(AppMode::item_add_shown( + cat_name, &shown, + ))] } else { vec![effect::refuse( "No category selected. Press n to add a category first.", @@ -790,7 +846,10 @@ impl Cmd for FilterToItem { category: cat_name.clone(), item: item_name.clone(), }), - effect::set_status(format!("Filter: {cat_name} = {item_name}")), + effect::set_status(filter_status( + ctx.model.category_display(&cat_name), + ctx.model.item_display(&cat_name, &item_name), + )), ] } Some(CatTreeEntry::Category { .. }) => { @@ -816,7 +875,7 @@ impl Cmd for DeleteCategoryAtCursor { Some(CatTreeEntry::Category { name, .. }) => { vec![ Box::new(effect::RemoveCategory(name.clone())), - effect::set_status(format!("Deleted category '{name}'")), + effect::set_status(deleted_category_status(ctx.model.category_display(&name))), ] } Some(CatTreeEntry::Item { @@ -829,7 +888,10 @@ impl Cmd for DeleteCategoryAtCursor { category: cat_name.clone(), item: item_name.clone(), }), - effect::set_status(format!("Deleted item '{item_name}' from '{cat_name}'")), + effect::set_status(deleted_item_status( + ctx.model.item_display(&cat_name, &item_name), + ctx.model.category_display(&cat_name), + )), ]; effects.extend(retreat_from_deleted_item(ctx)); effects diff --git a/crates/improvise-command/src/command/cmd/tile.rs b/crates/improvise-command/src/command/cmd/tile.rs index c4a71cf9..ceb197ee 100644 --- a/crates/improvise-command/src/command/cmd/tile.rs +++ b/crates/improvise-command/src/command/cmd/tile.rs @@ -8,6 +8,28 @@ mod tests { use super::*; use crate::command::cmd::test_helpers::*; + #[test] + fn a_tile_status_names_the_display_not_the_key() { + let mut m = two_cat_model(); + m.model + .category_mut("Type") + .unwrap() + .set_display("Kind of food and drink"); + let layout = make_layout(&m); + let reg = make_registry(); + let mut ctx = make_ctx(&m, &layout, ®); + ctx.tile_cat_idx = ctx + .view + .tileable_cats(&ctx.model.category_names()) + .iter() + .position(|c| *c == "Type") + .unwrap(); + let effects = TileAxisOp { axis: None }.execute(&ctx); + let said = format!("{:?}", effects.last().unwrap()); + assert!(said.contains("Kind of food and drink → "), "{said}"); + assert!(!said.contains("Type →"), "the key was said: {said}"); + } + #[test] fn tile_axis_cycle_produces_cycle_effect() { let m = two_cat_model(); @@ -161,7 +183,10 @@ impl Cmd for TileAxisOp { }), None => Box::new(effect::CycleAxis(ViewTarget::Active, name.to_string())), }; - let status = format!("{} → {}", name, new_axis.plain_label()); + let status = improvise_protocol::viewmodel::axis_change_status( + ctx.model.category_display(name), + new_axis, + ); vec![axis_effect, effect::set_status(status)] } else { vec![] diff --git a/crates/improvise-command/src/ui/app.rs b/crates/improvise-command/src/ui/app.rs index ce17b26d..fa398792 100644 --- a/crates/improvise-command/src/ui/app.rs +++ b/crates/improvise-command/src/ui/app.rs @@ -332,12 +332,9 @@ impl Grids<'_> { pub fn unwritable(&self, view: &str, cell: &RecordCell) -> Option { let key = self.resolve_cell(view, cell)?; let model = &self.app.model_state.workbook.model; - let derived = model.formula_for(&key).map(|f| { - format!( - "{} is computed by a formula — edit the formula, not the cell", - f.target - ) - }); + let derived = model + .formula_for(&key) + .map(|f| crate::command::cmd::core::derived_refusal(model, &f.target)); let refusal = self.app.non_item_refusal(&key).or(derived); refusal.or_else(|| self.aggregate_refusal(view, cell.record, &key)) } @@ -772,9 +769,9 @@ impl App { /// (improvise-b114, improvise-nacd). See `context/code/command.md`. fn non_item_refusal(&self, key: &CellKey) -> Option { let model = &self.model_state.workbook.model; - model - .first_non_item(key) - .map(|(cat, name)| improvise_protocol::non_item_refusal(cat, name)) + model.first_non_item(key).map(|(cat, name)| { + improvise_protocol::non_item_refusal(model.category_display(cat), name) + }) } /// Point **this client's** slice at `name`, if the document has such a view, @@ -5620,6 +5617,22 @@ mod non_item_key_tests { assert_eq!(app.view_state.status_msg, refusal("Mars")); } + #[test] + fn a_non_item_refusal_names_the_categorys_display() { + let mut app = paged_app("Mars"); + app.model_state + .workbook + .model + .category_mut("Region") + .unwrap() + .set_display("Sales territory of the whole company"); + press(&mut app, "i"); + assert_eq!( + app.view_state.status_msg, + improvise_protocol::non_item_refusal("Sales territory of the whole company", "Mars"), + ); + } + /// improvise-yc9r: `p` on a non-item cell writes no orphan. Evidence file. #[test] fn p_on_a_non_item_cell_writes_nothing() { diff --git a/crates/improvise-protocol/src/lib.rs b/crates/improvise-protocol/src/lib.rs index b9b880e4..6ac68f17 100644 --- a/crates/improvise-protocol/src/lib.rs +++ b/crates/improvise-protocol/src/lib.rs @@ -402,7 +402,14 @@ impl AppMode { } pub fn item_add(category: String) -> Self { - let prompt = format!("add item to {category}: "); + let shown = category.clone(); + Self::item_add_shown(category, &shown) + } + + /// The prompt names the category by what a person sees; `category` stays the + /// key the commit adds the item to (improvise-cq1q.13). + pub fn item_add_shown(category: String, shown: &str) -> Self { + let prompt = format!("add item to {shown}: "); Self::ItemAdd { category, minibuf: MinibufferConfig { diff --git a/crates/improvise-protocol/src/render.rs b/crates/improvise-protocol/src/render.rs index 10420abc..fd1c7084 100644 --- a/crates/improvise-protocol/src/render.rs +++ b/crates/improvise-protocol/src/render.rs @@ -70,6 +70,8 @@ pub enum CellId { /// Why a cell whose coordinate names no item takes no write. One wording for the /// projection's decline and the host's refusal (improvise-gk51). +/// `category` is what a person sees for it; `item` is the stray coordinate as +/// stored, which has no display because it names no item. pub fn non_item_refusal(category: &str, item: &str) -> String { format!("{item} is no item of {category}, so there is no one cell to write") } @@ -838,6 +840,19 @@ impl CategoryTreeCache { pub fn category(&self, name: &str) -> Option<&CategoryRow> { self.categories.iter().find(|c| c.name == name) } + + /// What a person sees for a category key; the key when the cache lacks it. + /// For a message only: never pass the result to a lookup (core.md §21). + pub fn category_display<'a>(&'a self, name: &'a str) -> &'a str { + self.category(name).map_or(name, |c| c.display.as_str()) + } + + /// What a person sees for an item of `cat`; the key when either is absent. + pub fn item_display<'a>(&'a self, cat: &str, item: &'a str) -> &'a str { + self.category(cat) + .and_then(|c| c.items.iter().find(|i| i.name == item)) + .map_or(item, |i| i.display.as_str()) + } } /// Three mutations, because the effects are three *shapes*. `Items` and @@ -955,6 +970,17 @@ pub struct ViewListCache { pub views: Vec, } +impl ViewListCache { + /// What a person sees for a view key; the key when the list lacks it. + /// For a title or a message only, never a lookup (core.md §21). + pub fn display_of<'a>(&'a self, key: &'a str) -> &'a str { + self.views + .iter() + .find(|v| v.name == key) + .map_or(key, |v| v.display.as_str()) + } +} + impl WidgetCache for ViewListCache { type Update = Vec; diff --git a/crates/improvise-protocol/src/resolve.rs b/crates/improvise-protocol/src/resolve.rs index 47f9befc..f5b75a3f 100644 --- a/crates/improvise-protocol/src/resolve.rs +++ b/crates/improvise-protocol/src/resolve.rs @@ -7,7 +7,10 @@ use serde::{Deserialize, Serialize}; use crate::client::{ClientState, Opened, OpenedCell, OpenedTarget}; use crate::frame::ClientFrame; use crate::keymap::{Binding, Keymaps, ModeKey}; -use crate::viewmodel::{CatTreeEntry, axis_change_status, flatten_cat_tree}; +use crate::viewmodel::{ + CatTreeEntry, axis_change_status, deleted_category_status, deleted_item_status, filter_status, + flatten_cat_tree, +}; use crate::{ AppMode, CellId, CellValue, Command, CreateKind, FormulaId, FormulaSlot, GridExtent, HelpPage, KeyCode, KeyEvent, ModelCommand, Panel, RECORD_COORDS_CANNOT_BE_EMPTY, RecordCell, ViewState, @@ -579,8 +582,9 @@ fn composed( category: cat.clone(), }, )?]; + let shown = state.categories.category_display(&cat); commands - .extend(cycled.map(|axis| Command::SetStatus(axis_change_status(&cat, axis)))); + .extend(cycled.map(|axis| Command::SetStatus(axis_change_status(shown, axis)))); commands } None => vec![Command::SetStatus( @@ -592,7 +596,10 @@ fn composed( "delete-formula-at-cursor" => delete_formula_at_cursor(state), "open-item-add-at-cursor" => Some(match cat_at_cursor(state) { - Some(cat) => vec![Command::ChangeMode(AppMode::item_add(cat))], + Some(cat) => { + let shown = state.categories.category_display(&cat).to_string(); + vec![Command::ChangeMode(AppMode::item_add_shown(cat, &shown))] + } None => vec![Command::SetStatus( "No category selected. Press n to add a category first.".to_string(), )], @@ -956,12 +963,14 @@ fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option> { "an aggregate is edited by drilling in — {}", not_yet("drill") ))]), - CellId::Derived { target } => Some(vec![Command::SetStatus(derived_decline(target))]), + CellId::Derived { target } => { + Some(vec![Command::SetStatus(derived_decline(state, target))]) + } // Both creating columns are records-only, so the guard above takes // them; declining rather than `None` keeps that a door, not a hole. CellId::Create(_) => Some(vec![Command::SetStatus(not_yet("creating a column"))]), CellId::NonItem { category, item } => Some(vec![Command::SetStatus( - crate::non_item_refusal(category, item), + crate::non_item_refusal(state.categories.category_display(category), item), )]), } } @@ -1399,8 +1408,11 @@ fn reopened_at( pub(crate) fn declined_at(state: &ClientState, row: usize, col: usize) -> Option { match projected_at(state, row, col)?.id.as_ref()? { CellId::Aggregate(_) => Some(AGGREGATE_DECLINE.to_string()), - CellId::Derived { target } => Some(derived_decline(target)), - CellId::NonItem { category, item } => Some(crate::non_item_refusal(category, item)), + CellId::Derived { target } => Some(derived_decline(state, target)), + CellId::NonItem { category, item } => Some(crate::non_item_refusal( + state.categories.category_display(category), + item, + )), CellId::Record(_) | CellId::Create(_) | CellId::Coordinate { .. } => None, } } @@ -1433,7 +1445,8 @@ const AGGREGATE_DECLINE: &str = "Cannot write to an aggregate — it combines several records, so there is no one cell to write"; /// What both ends say of a formula's result: the host's `derived_refusal`. -fn derived_decline(target: &str) -> String { +fn derived_decline(state: &ClientState, target: &str) -> String { + let target = state.categories.item_display("_Measure", target); format!("{target} is computed by a formula — edit the formula, not the cell") } @@ -1613,7 +1626,7 @@ fn filter_to_item_at(state: &ClientState, index: usize) -> Option> Some(CatTreeEntry::Item { cat_name, item_name, - .. + display, }) => Some(vec![ model( state, @@ -1631,7 +1644,10 @@ fn filter_to_item_at(state: &ClientState, index: usize) -> Option> item: item_name.clone(), }, )?, - Command::SetStatus(format!("Filter: {cat_name} = {item_name}")), + Command::SetStatus(filter_status( + state.categories.category_display(&cat_name), + &display, + )), ]), Some(CatTreeEntry::Category { .. }) => Some(toggle_cat_expand_at(state, index)), None => Some(Vec::new()), @@ -1642,14 +1658,14 @@ fn filter_to_item_at(state: &ClientState, index: usize) -> Option> /// row deletes the item, and an empty tree says so. fn delete_at_cat_cursor(state: &ClientState) -> Option> { match cat_tree_entry(state) { - Some(CatTreeEntry::Category { name, .. }) => Some(vec![ + Some(CatTreeEntry::Category { name, display, .. }) => Some(vec![ model(state, ModelCommand::RemoveCategory(name.clone()))?, - Command::SetStatus(format!("Deleted category '{name}'")), + Command::SetStatus(deleted_category_status(&display)), ]), Some(CatTreeEntry::Item { cat_name, item_name, - .. + display, }) => { let mut commands = vec![ model( @@ -1659,7 +1675,10 @@ fn delete_at_cat_cursor(state: &ClientState) -> Option> { item: item_name.clone(), }, )?, - Command::SetStatus(format!("Deleted item '{item_name}' from '{cat_name}'")), + Command::SetStatus(deleted_item_status( + &display, + state.categories.category_display(&cat_name), + )), ]; commands.extend(retreat_from_deleted_item(state)); Some(commands) @@ -1786,7 +1805,7 @@ fn tile_axis_at(state: &ClientState, index: usize, axis: Option) -> Option }; Some(vec![ model(state, cmd)?, - Command::SetStatus(axis_change_status(&tile.category, new_axis)), + Command::SetStatus(axis_change_status(&tile.display, new_axis)), ]) } @@ -5014,3 +5033,137 @@ mod opened_editor_wire_tests { ); } } + +#[cfg(test)] +mod display_tests { + use super::*; + use crate::{ + AxisFrame, CategoryItem, CategoryRow, CategoryTreeUpdate, CellDisplay, GridExtent, + GridUpdate, KeyModifiers, Projection, Snapshot, TileEntry, ViewState, + }; + + const REGION: &str = "Sales territory of the whole company"; + const NORTH: &str = "Northern reaches and the colder coasts"; + const TOTAL: &str = "Grand total of everything counted"; + + fn ev(code: KeyCode) -> KeyEvent { + KeyEvent::new(code, KeyModifiers::NONE) + } + + fn shown(name: &str, display: &str) -> CategoryItem { + CategoryItem { + name: name.into(), + display: display.into(), + } + } + + /// Every key differs from, and is far shorter than, what a person sees. + fn displayed(cell: Option) -> ClientState { + let mut state = ClientState::default(); + let mut first = CellDisplay::number("1"); + first.id = cell; + state.bootstrap(Snapshot { + view: ViewState::default(), + seed: vec![ + Projection::CategoryTree(CategoryTreeUpdate::Categories(vec![ + CategoryRow { + name: "Region".into(), + display: REGION.into(), + items: vec![shown("North", NORTH)], + axis: Axis::Row, + }, + CategoryRow { + name: "_Measure".into(), + display: "_Measure".into(), + items: vec![shown("Total", TOTAL)], + axis: Axis::Column, + }, + ])), + Projection::TileBar(vec![TileEntry { + category: "Region".into(), + display: REGION.into(), + axis: Axis::Row, + kind: crate::CategoryKind::Regular, + }]), + Projection::Grid(GridUpdate::RowFrame(AxisFrame::flat(["North"]))), + Projection::Grid(GridUpdate::ColFrame(AxisFrame::flat(["Total"]))), + Projection::Grid(GridUpdate::Cells(vec![((0, 0), first)])), + Projection::Grid(GridUpdate::Extent(GridExtent { rows: 1, cols: 1 })), + Projection::Basis(crate::basis::published(7, 3, Default::default())), + ], + }); + state.window = (1, 1); + state + } + + fn last_status(commands: Vec) -> String { + match commands.last() { + Some(Command::SetStatus(said)) => said.clone(), + other => panic!("no status last in {commands:?}: {other:?}"), + } + } + + fn local(state: &ClientState, code: KeyCode) -> Vec { + match resolve(state, ev(code)) { + KeyOutcome::Local(commands) => commands, + other => panic!("{code:?} did not resolve locally: {other:?}"), + } + } + + #[test] + fn a_panel_status_names_what_a_person_sees_not_the_keys() { + let mut state = displayed(None); + state.view.mode = AppMode::CategoryPanel; + state.view.expanded_cats.insert("Region".into()); + state.view.cat_panel_cursor = 0; + let cycled = last_status(local(&state, KeyCode::Char(' '))); + assert_eq!(cycled, format!("{REGION} → Col")); + let deleted = last_status(delete_at_cat_cursor(&state).unwrap()); + assert_eq!(deleted, format!("Deleted category '{REGION}'")); + + state.view.cat_panel_cursor = 1; + let filtered = last_status(filter_to_item(&state).unwrap()); + assert_eq!(filtered, format!("Filter: {REGION} = {NORTH}")); + let deleted = last_status(delete_at_cat_cursor(&state).unwrap()); + assert_eq!(deleted, format!("Deleted item '{NORTH}' from '{REGION}'")); + } + + #[test] + fn a_tile_status_names_the_display() { + let mut state = displayed(None); + state.view.mode = AppMode::TileSelect; + let said = last_status(tile_axis(&state, None).unwrap()); + assert_eq!(said, format!("{REGION} → Col")); + } + + #[test] + fn the_item_prompt_names_the_display_and_the_mode_keeps_the_key() { + let mut state = displayed(None); + state.view.mode = AppMode::CategoryPanel; + state.view.cat_panel_cursor = 0; + let commands = local(&state, KeyCode::Char('a')); + let Some(Command::ChangeMode(AppMode::ItemAdd { category, minibuf })) = commands + .iter() + .find(|c| matches!(c, Command::ChangeMode(_))) + else { + panic!("no item-add mode in {commands:?}"); + }; + assert_eq!(category, "Region", "the commit adds to the KEY"); + assert_eq!(minibuf.prompt, format!("add item to {REGION}: ")); + } + + #[test] + fn a_refused_cell_names_the_displays_it_shows() { + let derived = displayed(Some(CellId::Derived { + target: "Total".into(), + })); + let said = last_status(local(&derived, KeyCode::Char('i'))); + assert!(said.starts_with(TOTAL), "{said}"); + let stray = displayed(Some(CellId::NonItem { + category: "Region".into(), + item: "Mars".into(), + })); + let said = last_status(local(&stray, KeyCode::Char('i'))); + assert!(said.contains(REGION) && said.starts_with("Mars "), "{said}"); + } +} diff --git a/crates/improvise-protocol/src/viewmodel.rs b/crates/improvise-protocol/src/viewmodel.rs index 0de1255d..2d456775 100644 --- a/crates/improvise-protocol/src/viewmodel.rs +++ b/crates/improvise-protocol/src/viewmodel.rs @@ -38,11 +38,27 @@ impl CatTreeEntry { } /// What a moved axis says on the status line. One home: the tile bar and the -/// category panel are two routes to one operation — improvise-25bf. +/// category panel are two routes to one operation — improvise-25bf. Every +/// name here is a display: the status is read by a person, never matched on. pub fn axis_change_status(category: &str, axis: Axis) -> String { format!("{category} → {}", axis.plain_label()) } +/// What `filter-to-item` says; both ends, so one wording (improvise-cq1q.13). +pub fn filter_status(category: &str, item: &str) -> String { + format!("Filter: {category} = {item}") +} + +/// What deleting a category at the panel cursor says. +pub fn deleted_category_status(category: &str) -> String { + format!("Deleted category '{category}'") +} + +/// What deleting an item at the panel cursor says. +pub fn deleted_item_status(item: &str, category: &str) -> String { + format!("Deleted item '{item}' from '{category}'") +} + /// Flatten the category panel: a header per category, then its items when /// expanded. The result index is `ViewState::cat_panel_cursor`. pub fn flatten_cat_tree( diff --git a/crates/improvise-wasm-client/src/render.rs b/crates/improvise-wasm-client/src/render.rs index 9b32ec1d..1f6396cd 100644 --- a/crates/improvise-wasm-client/src/render.rs +++ b/crates/improvise-wasm-client/src/render.rs @@ -243,7 +243,7 @@ pub fn render_model(state: &ClientState) -> RenderModel { mode: improvise_protocol::mode_label(&state.view.mode).to_string(), status: state.view.status_msg.clone(), status_stands: state.status_stands(), - view_name: state.view.active_view.clone(), + view_name: state.views.display_of(&state.view.active_view).to_string(), doc_name: state.document.name.clone(), doc_dirty: state.document.dirty, doc_stored_as: state.document.stored_as.clone(), @@ -883,6 +883,24 @@ mod tests { assert!(views[0].active, "the active view is still found by its key"); } + #[test] + fn the_title_names_the_active_views_display_not_its_key() { + let mut client = init(); + let state = client.state_mut(); + state.fold(&Command::Project(Projection::ViewList(vec![ViewSummary { + name: "Default".into(), + display: "Quarterly overview of regional sales".into(), + axes: vec![], + }]))); + state.view.active_view = "Default".into(); + assert_eq!( + client.model().view_name, + "Quarterly overview of regional sales" + ); + client.state_mut().view.active_view = "Unlisted".into(); + assert_eq!(client.model().view_name, "Unlisted", "no display: the key"); + } + #[test] fn the_grid_page_bar_and_item_rows_paint_displays() { use improvise_protocol::{AxisFrame, CategoryItem, FrameGroup}; diff --git a/src/draw.rs b/src/draw.rs index af5a86e7..67e914d9 100644 --- a/src/draw.rs +++ b/src/draw.rs @@ -409,7 +409,7 @@ fn draw_content(f: &mut Frame, area: Rect, app: &App) { GridWidget::new( &grid_cache, app.cursor(), - &app.view_state.active_view, + app.view().display(), &app.layout, &app.view_state.mode, &app.view_state.search_query, @@ -1201,6 +1201,30 @@ mod content_tests { .collect() } + /// improvise-cq1q.13: evidence in `context/test-evidence/core.md`. + #[test] + fn the_grid_title_names_the_views_display_not_its_key() { + let mut wb = wide_grid(); + wb.active_view_mut() + .set_display("Quarterly overview of regional sales"); + let mut app = App::new(wb, None); + (app.term_width, app.term_height) = (100, 24); + let mut terminal = Terminal::new(TestBackend::new(100, 24)).expect("test backend"); + terminal.draw(|f| draw(f, &app)).expect("draw"); + let text: String = terminal + .backend() + .buffer() + .content() + .iter() + .map(|c| c.symbol()) + .collect(); + assert!( + text.contains("View: Quarterly overview of regional sales"), + "the title drew something else" + ); + assert!(!text.contains("View: Default"), "the key was drawn"); + } + /// improvise-gtn7.1: the cursor went behind a side panel — /// `context/test-evidence/tui.md`. #[test] diff --git a/src/ui/grid.rs b/src/ui/grid.rs index 9e03a96a..d897d749 100644 --- a/src/ui/grid.rs +++ b/src/ui/grid.rs @@ -23,7 +23,8 @@ pub struct GridWidget<'a> { /// This client's cursor in the active view (improvise-8uz). Passed in /// rather than read off `View`, which no longer carries per-client state. pub cursor: ViewCursor, - pub view_name: &'a str, + /// What the title shows: the view's display, never its key (improvise-cq1q.13). + pub view_display: &'a str, pub layout: &'a GridLayout, pub mode: &'a AppMode, pub search_query: &'a str, @@ -36,7 +37,7 @@ impl<'a> GridWidget<'a> { pub fn new( cache: &'a GridCache, cursor: ViewCursor, - view_name: &'a str, + view_display: &'a str, layout: &'a GridLayout, mode: &'a AppMode, search_query: &'a str, @@ -46,7 +47,7 @@ impl<'a> GridWidget<'a> { Self { cache, cursor, - view_name, + view_display, layout, mode, search_query, @@ -444,7 +445,7 @@ impl<'a> Widget for GridWidget<'a> { fn render(self, area: Rect, buf: &mut Buffer) { let block = Block::default() .borders(Borders::ALL) - .title(format!(" View: {} ", self.view_name)); + .title(format!(" View: {} ", self.view_display)); let inner = block.inner(area); block.render(area, buf); From 2e1f0f65508f4a18adfcfbf11ad99b3b6f28dcce Mon Sep 17 00:00:00 2001 From: "Claude (adr007-slug)" Date: Fri, 9 Oct 2026 12:49:09 -0700 Subject: [PATCH 03/11] docs(context): core.md section 20 and test evidence for the amended id rules (improvise-cq1q.14) Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- context/code/core.md | 89 ++++++++++++++------------ context/test-evidence/core.md | 92 +++++++++++++++++++++++---- crates/improvise-core/src/model/id.rs | 16 ++--- 3 files changed, 136 insertions(+), 61 deletions(-) diff --git a/context/code/core.md b/context/code/core.md index c00deb72..67a004ef 100644 --- a/context/code/core.md +++ b/context/code/core.md @@ -1688,16 +1688,16 @@ probability (`> ||[||]` is a self-parent), so `generated_file_parses` accepts a ## 20. How an id is made — `model/id.rs` (ADR 007 clause 4, improvise-cq1q.2) -`slugify` and `IdAllocator` are the first piece of ADR 007. They are pure -functions of the display and the allocator's own history. Nothing calls them -yet: the creation sites move onto them in the re-key step (improvise-cq1q.4). +`slugify`, `IdAllocator`, `natural_cmp` and `display_suffixes` are the first +piece of ADR 007. They are pure functions of their arguments and the +allocator's own history. Nothing calls them yet: the creation sites move onto them in the re-key step (improvise-cq1q.4). Each rule traces to ADR 007's clause 4 (slugify, the counter, hand-written ids) or clause 1 (reserved ids). **Case folding is full Unicode folding, and it comes from `unicase`.** -`str::to_lowercase` is not folding: `ß` lowercases to itself and is then -stripped, so `Straße` would slug to `strae` (clause 4, step 1: full case folding). The same goes for `ſ` -(long s) and the `fi` ligature. `unicase` 2.9 is `no_std` and has no +`str::to_lowercase` is not folding: `ß` lowercases to itself and then +becomes `_`, so `Straße` would slug to `stra_e` (clause 4, step 1: full case +folding). The same goes for `ſ` (long s) and the `fi` ligature. `unicase` 2.9 is `no_std` and has no dependencies, so it passes the wasm gate. Its `to_folded_case` applies the C+F mappings of Unicode's CaseFolding.txt, and its own tests pin `Maße` → `masse`. Most of what folding produces is stripped anyway: only an ASCII @@ -1705,31 +1705,45 @@ letter survives step 3. Deriving that ASCII-producing subset of the table by hand was rejected. It is a second copy of the Unicode data, and nothing would say when it drifted. -**A "space" is U+0020.** The accepted clause 4 says "replace each run of spaces -with one `_`". A tab or a no-break space is not a space here, so it is -stripped by step 3. Only a run of U+0020 becomes one `_`. An underscore the -display already had is never merged with the one a space run makes. So `a_ b` -slugs to `a__b`: the steps are applied as written, and no extra -underscore-collapsing step exists. A trailing run survives, so `East ` slugs -to `east_`. Only *leading* underscores are removed (step 4). +**Slugify is the amendment's six steps** (ADR 007 amendment 1, clause 4): +casefold; every character outside `a`–`z`, `0`–`9` and `-` becomes `_`; runs of +`_` collapse; leading and trailing `_` and `-` are removed, all of them (the +trim is `trim_matches` over both characters, so `_-_East-_-` is `east`); +`n` is prepended to a slug that starts with a digit; nothing left is the +caller's `-`. The order matters in one place: the trim runs before +the `n`, so `-_7up` is `n7up` and a leading `-` can never be hidden behind the +prefix. Whitespace of every kind, punctuation and every non-ASCII letter +become `_`, so `Region(code)` is `region_code` and `Crème Brûlée` is +`cr_me_br_l_e`. Folding is `unicase`'s, which maps `ß` to `ss`; a character +that folds to a combining mark (`İ` folds to `i` plus U+0307) leaves the `i` +and turns the mark into `_`, which the trim then removes. A hyphen between +underscores survives (`a - b` is `a_-_b`). -**ADR 007's amendment changes these rules, and this code predates it.** The -amendment (`context/adr/007-stable-identities-amendment-1.md`, accepted 2026-10-09) -says in its clause 4: -- every character outside `a`–`z`, `0`–`9` and `-` becomes `_` instead of being - stripped (not only U+0020), so `Region(code)` slugs to `region_code`, while - `Q1-2024` keeps its `-`; -- runs of `_` collapse, so `a_ b` and `a__b` both slug to `a_b`; -- leading and trailing `_` and `-` are removed, so `East ` → `east`, `-East-` → - `east`, and `-` alone takes `-`; -- `n` is prepended to a slug that starts with a digit, so `2024` → `n2024`; -- a hand-written id must obey every syntax rule of a generated one. +**The id syntax has one validator, `is_id_syntax`**: a lower-case ASCII +letter, then `a`–`z`, `0`–`9`, `-` and single underscores, never ending in +`_` or `-`. Slugify's non-empty output always satisfies it and is a fixed point +of itself, and a `-` suffix keeps the shape, so a generated id always +validates (properties in `model::id::tests`). A virtual id fails it by design, +because the leading `_` is the reservation. -This section, `slugify`, and the tests -`slugify_collapses_a_run_of_spaces_to_one_underscore` (its `east_` case) and -`slugify_keeps_an_underscore_the_display_already_had` describe the earlier rule -above. They change in a follow-up the amendment calls for. The amendment's -clause 4 lists every difference. +**Natural order is `natural_cmp`** (clause 4, "Ids are always sorted in +natural order"). An id is split into alternating digit and non-digit runs, and +the runs are compared pairwise: non-digit runs by bytes; digit runs by value +(strip leading zeros, shorter is smaller, then bytes), and a tie on value goes +to the run with fewer leading zeros, immediately and not after the later runs. +The id that runs out of runs first sorts first. No digit run is parsed into an +integer, so a run of any length compares correctly. Where two ids disagree +about the kind of run at a position (only possible for text that is not an +id), the runs compare as bytes. It is a total order and equal only for equal +strings. **Nothing calls it yet**: `CellKey`, `DataStore`, the writer and the GP +dataset still sort with `String::cmp` until the re-key step. + +**Equal displays are suffixed by `display_suffixes`** (clause 2). Given +`(id, display)` pairs it returns what each is shown as, in input order: the +first of a group of equal displays in natural id order keeps the bare display, +the rest get ` #2`, ` #3`, …, and a number is skipped when a literal display +already reads ` #` anywhere in the scope. Displays are equal +byte for byte, not case-folded. It is pure and nothing renders through it yet. **The allocator records every id it admits and releases none.** That is the whole of retirement (clause 4, "within a session a removed id is never handed @@ -1750,17 +1764,14 @@ name happens to be free, because clause 4 spells the result `-`. **`claim` is the only door for a hand-written id.** It checks, in this order: -the alphabet (non-empty, `a`–`z`, `0`–`9`, `-`, `_`), the reserved leading -`_` (only `_index`, `_dim` and `_measure` may carry one, per clause 1, and -the match is exact: `_indexer` and `_measure-1` are Reserved, since "no other -id may start with `_`"), and -duplicates. It accepts `-`, `-x`, `x-`, `a__b` and `2024`, all of which the -amendment refuses (a hand-written id obeys the syntax of a generated one). It never renames. -Refusing is the loader's job, and the loader +the reserved leading `_` (only `_index`, `_dim`, `_measure`, `_records` and +`_drill` may carry one, and the match is exact: `_indexer` and `_measure-1` +are Reserved), the syntax (`is_id_syntax`; `-x`, `x-`, `2024`, `a__b` and `x_` +are Malformed, `east-1`, `item-4`, `a-7` and `q1_-_2024` pass), and +duplicates. A virtual id skips the syntax check, since it fails it by design. +It never renames. Refusing is the loader's job, and the loader must name both lines of a duplicate; `ClaimError` carries only the id, so the -loader keeps the line numbers. The virtual views `_records` and `_drill` are -not yet in `VIRTUAL_IDS`. improvise-p003 item 1 asks for them, and they arrive -with the ADR text fix (improvise-cq1q.1) rather than ahead of it. +loader keeps the line numbers. `IdKind::Group` spells `group-`. ## 21. A display beside the key (ADR 007 clause 2, improvise-cq1q.3) diff --git a/context/test-evidence/core.md b/context/test-evidence/core.md index d4ae4c48..b25328ca 100644 --- a/context/test-evidence/core.md +++ b/context/test-evidence/core.md @@ -1914,30 +1914,96 @@ identity on the view. header stays for the sibling quarter that has data. Rival: `prune_axis_items` keying open runs by level alone. This test and the `NESTED` enumeration fail. -## `model/id.rs` (ADR 007 clause 4, improvise-cq1q.2) +## `model/id.rs` (ADR 007 clause 4 and amendment 1, improvise-cq1q.2, improvise-cq1q.14) -These tests were written before `slugify` and `IdAllocator` had bodies. With -`todo!()` bodies all eleven failed. Each rival below was then injected into the -finished module, run against `cargo test -p improvise-core --lib model::id`, and +Each rival below was injected into the finished module, run against `cargo test -p improvise-core --lib model::id`, and reverted. The reasoning is `context/code/core.md` §20. ### model::id::tests::slugify_case_folds_rather_than_lowercases Rival: `display.to_lowercase()` in place of `unicase`'s full folding. Only -this test fails: `left: "strae"`, `right: "strasse"`. `ß` lowercases to itself -and step 3 strips it. `slugify_follows_the_adrs_examples` cannot see this rival, +this test fails: `ß` lowercases to itself and becomes `_`, so `stra_e` against +`strasse`. `slugify_follows_the_amendments_table` cannot see this rival, because each of its displays lowercases and folds alike. -### model::id::tests::slugify_collapses_a_run_of_spaces_to_one_underscore +### model::id::tests::slugify_follows_the_amendments_table -Rival: every space becomes `_`, with runs not collapsed. Only this test fails: -`"time___period"`. No other fixture has two spaces in a row. +Every row of the amendment's clause 4 table, plus `-`, `%` and the empty +display. Rivals, each of which fails this test: trimming only leading `_` +(`East_` stays), trimming `_` and not `-` (`-East-`), not collapsing runs of +`_` (`a__b`), not prepending `n` (`2024`), and stripping instead of replacing +(`Region(code)` is `regioncode`). The last also fails +`slugify_keeps_a_hyphen_between_underscores` and +`a_slug_taken_by_another_kind_takes_a_suffix`. -### model::id::tests::slugify_keeps_an_underscore_the_display_already_had +### model::id::tests::slugify_trims_hyphens_and_underscores_until_neither_is_at_an_end -Rival: `.replace("__", "_")` after the space step, which reads the accepted -clause 4 as "no doubled underscores". Only this test fails: `left: "a_b"`. The rule -collapses runs of *spaces*. An underscore that was in the display is kept. +Rival: trim leading `_` only, or `_` only at both ends. `_-_East-_-` keeps a +hyphen, and `-_-` is not empty. A single `trim_matches` over both characters +passes; two sequential single-character trims would not. + +### model::id::tests::slugify_trims_before_it_prepends_the_n + +Rival: no `n` prefix, which fails this test, the table and the property below. +`-_7up` is `n7up` only if the trim runs first; prepending first would leave the +`-_` in front of the `n`. + +### model::id::tests::slugify_output_has_the_id_syntax_and_is_a_fixed_point + +The property over arbitrary displays: a non-empty slug satisfies +`is_id_syntax` and slugifying it returns it. Rivals that fail it (and +`generated_ids_are_distinct_well_formed_and_never_reserved`): no `_` +collapse, no `n` prefix, keeping a trailing `_` or `-`. + +### model::id::tests::id_syntax_agrees_with_its_regular_expression + +`is_id_syntax` against the spelled-out rule over `[a-c0-9_-]{0,8}`. Rivals: +allowing `__`, allowing a leading digit and allowing a trailing `-` each fail +here and in `claim_refuses_what_a_generated_id_could_not_be`. + +### model::id::tests::claim_refuses_what_a_generated_id_could_not_be + +Rival: `claim` keeps the old alphabet-only check. `-x`, `x-`, `2024`, `a__b`, +`x_` and `-` are admitted, and the test fails. It also pins the accepted ids +`east-1`, `item-4`, `a-7` and `q1_-_2024`. + +### model::id::tests::claim_admits_each_virtual_id_once + +Rival: `VIRTUAL_IDS` without `_records` and `_drill`. The test spells the five +ids out instead of iterating the constant, because an iteration over the +constant passes with the constant shortened (the first version did). + +### model::id::tests::natural_order_puts_a_counter_suffix_in_numeric_order + +Rivals: comparing digit runs as strings (`profit-10` before `profit-2`) fails +this test, `natural_order_compares_digit_runs_of_any_length_by_value` and +`a_suffix_numbers_equal_displays_in_natural_id_order`; plain `str::cmp` fails +those and `natural_order_ties_on_value_go_to_fewer_leading_zeros`. + +### model::id::tests::natural_order_ties_on_value_go_to_fewer_leading_zeros + +Rivals: more zeros first, or no tie-break (`a-7` equal to `a-007`). Both fail this test and the any-length test (`x000` after `x0`). + +### model::id::tests::natural_order_puts_the_id_that_runs_out_of_runs_first + +Rival: the longer id first. Only this test fails. + +### model::id::tests::natural_order_is_a_total_order + +Antisymmetry, equality only for equal strings and transitivity over +`[a-c0-3_-]{0,7}`, which includes ids starting with a digit. + +### model::id::tests::a_suffix_numbers_equal_displays_in_natural_id_order + +Rival: ordering the group by `str::cmp` on the ids. `profit-10` is numbered +before `profit-2` and this test fails alone. + +### model::id::tests::a_suffix_skips_a_number_a_literal_display_already_holds + +Rival: numbering without the skip. A literal `Profit #2` and a generated +`Profit #2` are then both shown, and this test fails alone. A rival that starts +numbering at ` #1` also fails it and +`a_suffix_is_gapless_when_a_thing_goes_away`. ### model::id::tests::the_suffix_is_the_next_counter_value_not_the_lowest_free_number diff --git a/crates/improvise-core/src/model/id.rs b/crates/improvise-core/src/model/id.rs index 5340cc09..dfdea0f1 100644 --- a/crates/improvise-core/src/model/id.rs +++ b/crates/improvise-core/src/model/id.rs @@ -239,6 +239,7 @@ mod tests { assert_eq!(slugify("Straße"), "strasse"); assert_eq!(slugify("ſtate"), "state"); assert_eq!(slugify("file"), "file"); + assert_eq!(slugify("İ"), "i"); } #[test] @@ -352,7 +353,7 @@ mod tests { #[test] fn claim_admits_each_virtual_id_once() { let mut ids = IdAllocator::new(); - for v in VIRTUAL_IDS { + for v in ["_index", "_dim", "_measure", "_records", "_drill"] { assert_eq!(ids.claim(v), Ok(())); assert_eq!(ids.claim(v), Err(ClaimError::Duplicate(v.into()))); } @@ -447,13 +448,7 @@ mod tests { ]; assert_eq!( display_suffixes(&items), - [ - "Profit #4", - "Profit", - "Profit #3", - "Profit #2", - "Cost" - ] + ["Profit #4", "Profit", "Profit #3", "Profit #2", "Cost"] ); } @@ -474,7 +469,10 @@ mod tests { #[test] fn a_suffix_is_gapless_when_a_thing_goes_away() { let three = [("p", "Profit"), ("p-1", "Profit"), ("p-2", "Profit")]; - assert_eq!(display_suffixes(&three), ["Profit", "Profit #2", "Profit #3"]); + assert_eq!( + display_suffixes(&three), + ["Profit", "Profit #2", "Profit #3"] + ); let two = [("p", "Profit"), ("p-2", "Profit")]; assert_eq!(display_suffixes(&two), ["Profit", "Profit #2"]); } From bf7e999edcdbf185bdee69a22d6d8bef4e9f4de4 Mon Sep 17 00:00:00 2001 From: "Claude (adr007-2c)" Date: Fri, 9 Oct 2026 12:49:12 -0700 Subject: [PATCH 04/11] test(protocol): the filter and delete resolvers still name keys in what they command (improvise-cq1q.13) Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- crates/improvise-protocol/src/resolve.rs | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/crates/improvise-protocol/src/resolve.rs b/crates/improvise-protocol/src/resolve.rs index f5b75a3f..10dc4358 100644 --- a/crates/improvise-protocol/src/resolve.rs +++ b/crates/improvise-protocol/src/resolve.rs @@ -5122,9 +5122,28 @@ mod display_tests { assert_eq!(deleted, format!("Deleted category '{REGION}'")); state.view.cat_panel_cursor = 1; - let filtered = last_status(filter_to_item(&state).unwrap()); + let filtering = filter_to_item(&state).unwrap(); + let named: Vec = filtering + .iter() + .filter_map(|c| match c { + Command::Model { + cmd: ModelCommand::SetPageSelection { category, item, .. }, + .. + } => Some(format!("{category}/{item}")), + _ => None, + }) + .collect(); + assert_eq!(named, ["Region/North"], "the lookup names keys"); + let filtered = last_status(filtering); assert_eq!(filtered, format!("Filter: {REGION} = {NORTH}")); - let deleted = last_status(delete_at_cat_cursor(&state).unwrap()); + let removal = delete_at_cat_cursor(&state).unwrap(); + assert!( + removal.iter().any(|c| matches!(c, Command::Model { + cmd: ModelCommand::RemoveItem { category, item }, .. + } if category == "Region" && item == "North")), + "the removal names keys: {removal:?}" + ); + let deleted = last_status(removal); assert_eq!(deleted, format!("Deleted item '{NORTH}' from '{REGION}'")); } From 5f741ee1dae3cf7f98f564d39ad2c5731e404a69 Mon Sep 17 00:00:00 2001 From: "Claude (adr007-2c)" Date: Fri, 9 Oct 2026 12:51:20 -0700 Subject: [PATCH 05/11] test(command): the wire refusals and the p key name displays too (improvise-cq1q.13) Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- crates/improvise-command/src/ui/app.rs | 53 ++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/crates/improvise-command/src/ui/app.rs b/crates/improvise-command/src/ui/app.rs index fa398792..60fd36f1 100644 --- a/crates/improvise-command/src/ui/app.rs +++ b/crates/improvise-command/src/ui/app.rs @@ -5617,6 +5617,47 @@ mod non_item_key_tests { assert_eq!(app.view_state.status_msg, refusal("Mars")); } + #[test] + fn a_refused_wire_write_names_the_displays_it_shows() { + const REGION: &str = "Sales territory of the whole company"; + const PROFIT: &str = "Operating profit after all costs"; + let mut app = paged_app("East"); + let stray = CellKey::new(vec![ + ("Month".into(), "Jan".into()), + ("Region".into(), "Mars".into()), + ("_Measure".into(), "Revenue".into()), + ]); + let model = &mut app.model_state.workbook.model; + model.set_cell(stray.clone(), CellValue::Number(2.0)); + model.category_mut("Region").unwrap().set_display(REGION); + model.add_formula( + crate::formula::parse_formula("Profit = Revenue - 1", "_Measure").unwrap(), + ); + let measure = model.category_mut("_Measure").unwrap(); + measure.add_item("Profit"); + measure.set_item_display("Profit", PROFIT); + let refused = |key: &CellKey, measure: &str| { + let cell = RecordCell { + record: RecordHash::of(key), + measure: Some(measure.into()), + }; + app.unwritable(app.view_name(), &cell) + }; + let why = refused(&stray, "Revenue").expect("a non-item is refused"); + assert!(why.contains(REGION), "{why}"); + assert!( + why.starts_with("Mars "), + "the stray coordinate is a key: {why}" + ); + let profit = CellKey::new(vec![ + ("Month".into(), "Jan".into()), + ("Region".into(), "East".into()), + ("_Measure".into(), "Profit".into()), + ]); + let why = refused(&profit, "Profit").expect("a formula's result is refused"); + assert!(why.starts_with(PROFIT), "{why}"); + } + #[test] fn a_non_item_refusal_names_the_categorys_display() { let mut app = paged_app("Mars"); @@ -5631,6 +5672,18 @@ mod non_item_key_tests { app.view_state.status_msg, improvise_protocol::non_item_refusal("Sales territory of the whole company", "Mars"), ); + app.view_state.yanked = Some(CellValue::Number(7.0)); + press(&mut app, "p"); + assert!( + app.view_state + .status_msg + .ends_with(&improvise_protocol::non_item_refusal( + "Sales territory of the whole company", + "Mars" + )), + "{}", + app.view_state.status_msg + ); } /// improvise-yc9r: `p` on a non-item cell writes no orphan. Evidence file. From b17bcb068bef266c7033a75ea3f10129ccacbe15 Mon Sep 17 00:00:00 2001 From: "Claude (adr007-slug)" Date: Fri, 9 Oct 2026 12:55:14 -0700 Subject: [PATCH 06/11] docs(context): core.md section 20 traces its rules to all three clauses and says folding output becomes _ (improvise-cq1q.14) Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- context/code/core.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/context/code/core.md b/context/code/core.md index 67a004ef..24f123bb 100644 --- a/context/code/core.md +++ b/context/code/core.md @@ -1692,7 +1692,8 @@ probability (`> ||[||]` is a self-parent), so `generated_file_parses` accepts a piece of ADR 007. They are pure functions of their arguments and the allocator's own history. Nothing calls them yet: the creation sites move onto them in the re-key step (improvise-cq1q.4). Each rule traces to ADR 007's clause 4 (slugify, the counter, hand-written -ids) or clause 1 (reserved ids). +ids, natural order), clause 1 (reserved ids) or clause 2 (display suffixes), +as amended by amendment 1. **Case folding is full Unicode folding, and it comes from `unicase`.** `str::to_lowercase` is not folding: `ß` lowercases to itself and then @@ -1700,9 +1701,9 @@ becomes `_`, so `Straße` would slug to `stra_e` (clause 4, step 1: full case folding). The same goes for `ſ` (long s) and the `fi` ligature. `unicase` 2.9 is `no_std` and has no dependencies, so it passes the wasm gate. Its `to_folded_case` applies the C+F mappings of Unicode's CaseFolding.txt, and its own tests pin `Maße` → -`masse`. Most of what folding produces is stripped anyway: only an ASCII -letter survives step 3. Deriving that ASCII-producing subset of the table by -hand was rejected. It is a second copy of the Unicode data, and nothing would +`masse`. Most of what folding produces becomes `_` anyway: only ASCII +letters, digits and `-` survive step 2. Deriving that ASCII-producing subset of +the table by hand was rejected. It is a second copy of the Unicode data, and nothing would say when it drifted. **Slugify is the amendment's six steps** (ADR 007 amendment 1, clause 4): From bb7fe88547e9733cf954a9af1e0e6d9d23c24001 Mon Sep 17 00:00:00 2001 From: "Claude (adr007-2c)" Date: Fri, 9 Oct 2026 12:55:41 -0700 Subject: [PATCH 07/11] =?UTF-8?q?docs(context):=20core.md=20=C2=A721=20thi?= =?UTF-8?q?rd=20slice,=20test=20evidence,=20and=20two=20comment=20runs=20s?= =?UTF-8?q?hortened=20(improvise-cq1q.13)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- context/code/core.md | 30 +++++++-- context/test-evidence/core.md | 72 ++++++++++++++++++++++ crates/improvise-command/src/ui/app.rs | 15 +++++ crates/improvise-protocol/src/render.rs | 6 +- crates/improvise-protocol/src/viewmodel.rs | 5 +- 5 files changed, 116 insertions(+), 12 deletions(-) diff --git a/context/code/core.md b/context/code/core.md index 2b2023e6..3ea1b81e 100644 --- a/context/code/core.md +++ b/context/code/core.md @@ -1790,11 +1790,31 @@ beside the key and never in its place: - The browser's render model keeps its field names and carries the display in them (`RenderPageAxis.cat`, `RenderCatRow.label`), so `web/` is unchanged. -Still keys: `GridLayout::row_label`/`col_label` (commands and the status line -match on them), `edit_text`, the active-view marker, and everything status -messages and the view title say (improvise-cq1q.13). A display change alone does -not move `layout_fingerprint`, so it resets no cursor; no production path changes -one yet. +Still keys: `GridLayout::row_label`/`col_label` (commands match on them), +`edit_text` and the active-view marker. A display change alone does not move +`layout_fingerprint`, so it resets no cursor; no production path changes one yet. + +**What the third slice covers (improvise-cq1q.13):** the view title and every +message that names a thing which exists. The title is the active view's display: +the TUI reads `app.view().display()`, the browser `ViewListCache::display_of` +(key when the list lacks the view). The statuses are the axis change (panel and +tile bar), `Filter:`, `Deleted category/item`, the formula-result and non-item +refusals, and the item-add prompt. Both ends say them through the same +`viewmodel` builders, so a wording is not spelled twice. The host reads +`Model::category_display`/`item_display`; the client reads +`CategoryTreeCache::category_display`/`item_display`. Each falls back to the key. + +**A message that names something which does not exist says what was typed.** +`Unknown category 'x'`, `No view named "x"`, `View 'x' not found` and the +`workbook.rs` errors are lookup misses: there is no display to find, and the typed +text is what the person needs to correct. `Added category "x"` is optimistic and +names what was just typed, whose display equals its key at creation. A stray +coordinate in `non_item_refusal` is a key for the same reason; only the category +beside it is a display. **A display must never reach a lookup**: the item-add +mode keeps the category key (`AppMode::item_add_shown(key, shown)`) and the prompt +alone carries the display. A formula target has no display path yet +(`set_item_display` needs an explicit `_Measure` item), so `derived_refusal` reads +`item_display("_Measure", target)` and in practice says the key. `AxisFrame::cat_displays` is projected and validated, but nothing paints it yet (the corner label has no consumer); that is a gap to close when one does, not a diff --git a/context/test-evidence/core.md b/context/test-evidence/core.md index 38d50101..4fb6c30c 100644 --- a/context/test-evidence/core.md +++ b/context/test-evidence/core.md @@ -2143,3 +2143,75 @@ The display "Eastern Seaboard" against the key "E" gives 17, not the 4 minimum. Rival: `col_groups` labels a run with `g.name`. The browser test for group displays covered row groups only; this one folds a column frame whose group "Q1" is shown as "First quarter" and fails on the key. + +### draw::content_tests::the_grid_title_names_the_views_display_not_its_key + +improvise-cq1q.13, TUI half. The view is displayed "Quarterly overview of regional +sales" under the key "Default", and the whole frame is drawn through `draw`. Rival: +`draw_content` passes `app.view_state.active_view` to the widget; the title reads +`View: Default` and this test fails on both assertions. + +### render::tests::the_title_names_the_active_views_display_not_its_key + +The browser half. Rival: `render_model` takes `view_name` from +`state.view.active_view`; the test fails. The second half is the control: a view +the list does not hold falls back to its key rather than to an empty title. + +### resolve::display_tests::a_panel_status_names_what_a_person_sees_not_the_keys + +The browser resolver's panel statuses: axis cycle, filter, delete category, delete +item. Displays are 36 to 38 characters against keys of 4 to 6. Rivals, each run: +the cycle status passes the key; the filter composes `SetPageSelection` with the +item's display; the item removal names the display. Each fails here, the last two +on the `Region/North` and `RemoveItem` assertions that pin the lookup to keys. + +### resolve::display_tests::a_tile_status_names_the_display + +Rival: the tile route passes `tile.category`; fails on the key. + +### resolve::display_tests::the_item_prompt_names_the_display_and_the_mode_keeps_the_key + +Rival: the mode is built with the display as its category +(`item_add_shown(shown, ..)`); the prompt reads right and the commit would add to +a category that does not exist. The test fails on `category == "Region"`. + +### resolve::display_tests::a_refused_cell_names_the_displays_it_shows + +Rival: the non-item refusal gets the category key. The derived arm is covered by +the item display for `_Measure`; the stray coordinate stays a key because it names +no item and has no display. + +### command::cmd::panel::tests::a_panel_status_names_what_a_person_sees_not_the_keys + +The host half of the panel statuses and the item-add prompt. Rivals, each run: the +axis status passes `cat_name` (fails); the filter status passes the item key +(fails); the item-add mode is built from the display (fails on the +`category: "Type"` assertion). + +### command::cmd::tile::tests::a_tile_status_names_the_display_not_the_key + +Rival: the status formats `name`. It fails on both the present display and the +absent key. + +### command::cmd::cell::tests::a_refusal_names_the_formulas_display_not_its_key + +A formula target has no display path today: `set_item_display` needs an explicit +`_Measure` item, so the fixture adds one named like the target. Rival: +`derived_refusal` skips the lookup; the test fails on "Operating profit...". + +### ui::app::non_item_key_tests::a_non_item_refusal_names_the_categorys_display + +Rival, run once per site: the refusal in `EditOrDrill` (the `i` key) and the one in +`CmdContext::value_cell_key` (the `p` key) each pass the key. Each fails here. + +### ui::app::non_item_key_tests::a_commit_on_a_non_item_cell_names_the_categorys_display + +The third site, `uneditable` in `cmd/core.rs`, which only a commit reaches once an +editor is open (`:enter-mode editing`). Rival: it passes the key; the two tests +above stay green and this one fails. + +### ui::app::non_item_key_tests::a_refused_wire_write_names_the_displays_it_shows + +The wire's own refusals in `Grids::unwritable`: the non-item arm and the formula +arm (once a third copy of the derived wording, now `derived_refusal`). Rivals: the +category key; a model with no display for the formula. Each fails. diff --git a/crates/improvise-command/src/ui/app.rs b/crates/improvise-command/src/ui/app.rs index 60fd36f1..d655b163 100644 --- a/crates/improvise-command/src/ui/app.rs +++ b/crates/improvise-command/src/ui/app.rs @@ -5721,6 +5721,21 @@ mod non_item_key_tests { assert!(status.ends_with(&refusal("Mars")), "{status}"); } + #[test] + fn a_commit_on_a_non_item_cell_names_the_categorys_display() { + const REGION: &str = "Sales territory of the whole company"; + let mut app = paged_app("Mars"); + let region = app.model_state.workbook.model.category_mut("Region"); + region.unwrap().set_display(REGION); + press(&mut app, ":enter-mode editing\n"); + press(&mut app, "5\n"); + let said = &app.view_state.status_msg; + assert!( + said.ends_with(&improvise_protocol::non_item_refusal(REGION, "Mars")), + "{said}" + ); + } + /// `uneditable` declines a non-item cell, so an advance re-opens no editor /// on one (#265 meets #277). Evidence: `context/test-evidence/command.md`. #[test] diff --git a/crates/improvise-protocol/src/render.rs b/crates/improvise-protocol/src/render.rs index fd1c7084..8072f005 100644 --- a/crates/improvise-protocol/src/render.rs +++ b/crates/improvise-protocol/src/render.rs @@ -68,10 +68,8 @@ pub enum CellId { NonItem { category: String, item: String }, } -/// Why a cell whose coordinate names no item takes no write. One wording for the -/// projection's decline and the host's refusal (improvise-gk51). -/// `category` is what a person sees for it; `item` is the stray coordinate as -/// stored, which has no display because it names no item. +/// Why a cell naming no item takes no write (improvise-gk51). `category` is a +/// display; `item` is the stray key, which has none (`core.md` §21). pub fn non_item_refusal(category: &str, item: &str) -> String { format!("{item} is no item of {category}, so there is no one cell to write") } diff --git a/crates/improvise-protocol/src/viewmodel.rs b/crates/improvise-protocol/src/viewmodel.rs index 2d456775..bbb66012 100644 --- a/crates/improvise-protocol/src/viewmodel.rs +++ b/crates/improvise-protocol/src/viewmodel.rs @@ -37,9 +37,8 @@ impl CatTreeEntry { } } -/// What a moved axis says on the status line. One home: the tile bar and the -/// category panel are two routes to one operation — improvise-25bf. Every -/// name here is a display: the status is read by a person, never matched on. +/// What a moved axis says on the status line: one home for the tile bar and the +/// panel (improvise-25bf). Takes displays; `core.md` §21. pub fn axis_change_status(category: &str, axis: Axis) -> String { format!("{category} → {}", axis.plain_label()) } From 300b5afc7f55c8861356cc499752d173671c5889 Mon Sep 17 00:00:00 2001 From: "Claude (adr007-2c)" Date: Fri, 9 Oct 2026 13:08:33 -0700 Subject: [PATCH 08/11] fix(display): defer the formula-result display: the surrogate cannot find the target's category (improvise-cq1q.13) Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- context/code/core.md | 12 +++--- context/test-evidence/core.md | 21 ++++------- .../improvise-command/src/command/cmd/cell.rs | 18 --------- .../improvise-command/src/command/cmd/core.rs | 7 ++-- .../improvise-command/src/command/cmd/mode.rs | 1 - crates/improvise-command/src/ui/app.rs | 10 ++--- crates/improvise-protocol/src/resolve.rs | 37 +++++-------------- 7 files changed, 30 insertions(+), 76 deletions(-) diff --git a/context/code/core.md b/context/code/core.md index 3ea1b81e..5ece20d5 100644 --- a/context/code/core.md +++ b/context/code/core.md @@ -1798,8 +1798,8 @@ Still keys: `GridLayout::row_label`/`col_label` (commands match on them), message that names a thing which exists. The title is the active view's display: the TUI reads `app.view().display()`, the browser `ViewListCache::display_of` (key when the list lacks the view). The statuses are the axis change (panel and -tile bar), `Filter:`, `Deleted category/item`, the formula-result and non-item -refusals, and the item-add prompt. Both ends say them through the same +tile bar), `Filter:`, `Deleted category/item`, the non-item +refusal, and the item-add prompt. Both ends say them through the same `viewmodel` builders, so a wording is not spelled twice. The host reads `Model::category_display`/`item_display`; the client reads `CategoryTreeCache::category_display`/`item_display`. Each falls back to the key. @@ -1812,9 +1812,11 @@ names what was just typed, whose display equals its key at creation. A stray coordinate in `non_item_refusal` is a key for the same reason; only the category beside it is a display. **A display must never reach a lookup**: the item-add mode keeps the category key (`AppMode::item_add_shown(key, shown)`) and the prompt -alone carries the display. A formula target has no display path yet -(`set_item_display` needs an explicit `_Measure` item), so `derived_refusal` reads -`item_display("_Measure", target)` and in practice says the key. +alone carries the display. A formula-result refusal still says the target's key +(improvise-cq1q.13 deferred it). A target in `_Measure` is implied by its formula +and is no item, so it has no display to read. One in a regular category is a real +item, but `CellId::Derived` carries only the target, so neither end can find its +category. Both need that surrogate to carry the category first. `AxisFrame::cat_displays` is projected and validated, but nothing paints it yet (the corner label has no consumer); that is a gap to close when one does, not a diff --git a/context/test-evidence/core.md b/context/test-evidence/core.md index 4fb6c30c..6a446212 100644 --- a/context/test-evidence/core.md +++ b/context/test-evidence/core.md @@ -2175,11 +2175,10 @@ Rival: the mode is built with the display as its category (`item_add_shown(shown, ..)`); the prompt reads right and the commit would add to a category that does not exist. The test fails on `category == "Region"`. -### resolve::display_tests::a_refused_cell_names_the_displays_it_shows +### resolve::display_tests::a_refused_cell_names_the_category_it_shows -Rival: the non-item refusal gets the category key. The derived arm is covered by -the item display for `_Measure`; the stray coordinate stays a key because it names -no item and has no display. +Rival: the non-item refusal gets the category key. The stray coordinate stays a key +because it names no item and has no display. ### command::cmd::panel::tests::a_panel_status_names_what_a_person_sees_not_the_keys @@ -2193,12 +2192,6 @@ axis status passes `cat_name` (fails); the filter status passes the item key Rival: the status formats `name`. It fails on both the present display and the absent key. -### command::cmd::cell::tests::a_refusal_names_the_formulas_display_not_its_key - -A formula target has no display path today: `set_item_display` needs an explicit -`_Measure` item, so the fixture adds one named like the target. Rival: -`derived_refusal` skips the lookup; the test fails on "Operating profit...". - ### ui::app::non_item_key_tests::a_non_item_refusal_names_the_categorys_display Rival, run once per site: the refusal in `EditOrDrill` (the `i` key) and the one in @@ -2210,8 +2203,8 @@ The third site, `uneditable` in `cmd/core.rs`, which only a commit reaches once editor is open (`:enter-mode editing`). Rival: it passes the key; the two tests above stay green and this one fails. -### ui::app::non_item_key_tests::a_refused_wire_write_names_the_displays_it_shows +### ui::app::non_item_key_tests::a_refused_wire_write_names_the_category_it_shows -The wire's own refusals in `Grids::unwritable`: the non-item arm and the formula -arm (once a third copy of the derived wording, now `derived_refusal`). Rivals: the -category key; a model with no display for the formula. Each fails. +The wire's own refusal in `Grids::unwritable`. Rival: the category key; fails. The +formula arm (once a third copy of the derived wording, now `derived_refusal`) is +pinned to its wording so the consolidation cannot change what it says. diff --git a/crates/improvise-command/src/command/cmd/cell.rs b/crates/improvise-command/src/command/cmd/cell.rs index d3b43150..7bcb3112 100644 --- a/crates/improvise-command/src/command/cmd/cell.rs +++ b/crates/improvise-command/src/command/cmd/cell.rs @@ -494,24 +494,6 @@ mod tests { app } - #[test] - fn a_refusal_names_the_formulas_display_not_its_key() { - let mut app = formula_app(); - let measure = app.model_state.workbook.model.category_mut("_Measure"); - let measure = measure.unwrap(); - measure.add_item("Profit"); - measure.set_item_display("Profit", "Operating profit after all costs"); - app.cursor_mut().selected = (1, 2); - press(&mut app, 'p'); - assert!( - app.view_state - .status_msg - .contains("Operating profit after all costs is computed"), - "got {:?}", - app.view_state.status_msg - ); - } - /// improvise-54g1 — see context/test-evidence/command.md. #[test] fn p_onto_a_formula_owned_cell_writes_nothing_and_names_the_formula() { diff --git a/crates/improvise-command/src/command/cmd/core.rs b/crates/improvise-command/src/command/cmd/core.rs index 6491efc5..26cc90d8 100644 --- a/crates/improvise-command/src/command/cmd/core.rs +++ b/crates/improvise-command/src/command/cmd/core.rs @@ -23,8 +23,7 @@ pub const INSIDE_A_DRILL_REFUSAL: &str = "A value is not staged inside a drill — edit the cell instead"; /// What a command says when the cell it would write is a formula's result. -pub fn derived_refusal(model: &Model, target: &str) -> String { - let target = model.item_display("_Measure", target); +pub fn derived_refusal(target: &str) -> String { format!("{target} is computed by a formula — edit the formula, not the cell") } @@ -39,7 +38,7 @@ pub fn uneditable(model: &Model, layout: &GridLayout, row: usize, col: usize) -> } .to_string(), ), - Some(CellId::Derived { target }) => Some(derived_refusal(model, &target)), + Some(CellId::Derived { target }) => Some(derived_refusal(&target)), Some(CellId::NonItem { category, item }) => { Some(non_item_refusal(model.category_display(&category), &item)) } @@ -132,7 +131,7 @@ impl<'a> CmdContext<'a> { match self.cursor_cell_id() { Some(CellId::Record(_)) => self.stored_cell_key(), Some(CellId::Aggregate(_)) => Err(self.aggregate_refusal()), - Some(CellId::Derived { target }) => Err(derived_refusal(self.model, &target)), + Some(CellId::Derived { target }) => Err(derived_refusal(&target)), Some(CellId::Create(_) | CellId::Coordinate { .. }) => { Err(NO_VALUE_REFUSAL.to_string()) } diff --git a/crates/improvise-command/src/command/cmd/mode.rs b/crates/improvise-command/src/command/cmd/mode.rs index 9db0b1e2..35a535da 100644 --- a/crates/improvise-command/src/command/cmd/mode.rs +++ b/crates/improvise-command/src/command/cmd/mode.rs @@ -386,7 +386,6 @@ impl Cmd for EditOrDrill { // the synthetic key names no measure, so the record's key is resolved. if let Some(formula) = ctx.owning_formula() { return vec![effect::refuse(super::core::derived_refusal( - ctx.model, &formula.target, ))]; } diff --git a/crates/improvise-command/src/ui/app.rs b/crates/improvise-command/src/ui/app.rs index d655b163..87f02fd2 100644 --- a/crates/improvise-command/src/ui/app.rs +++ b/crates/improvise-command/src/ui/app.rs @@ -334,7 +334,7 @@ impl Grids<'_> { let model = &self.app.model_state.workbook.model; let derived = model .formula_for(&key) - .map(|f| crate::command::cmd::core::derived_refusal(model, &f.target)); + .map(|f| crate::command::cmd::core::derived_refusal(&f.target)); let refusal = self.app.non_item_refusal(&key).or(derived); refusal.or_else(|| self.aggregate_refusal(view, cell.record, &key)) } @@ -5618,9 +5618,8 @@ mod non_item_key_tests { } #[test] - fn a_refused_wire_write_names_the_displays_it_shows() { + fn a_refused_wire_write_names_the_category_it_shows() { const REGION: &str = "Sales territory of the whole company"; - const PROFIT: &str = "Operating profit after all costs"; let mut app = paged_app("East"); let stray = CellKey::new(vec![ ("Month".into(), "Jan".into()), @@ -5633,9 +5632,6 @@ mod non_item_key_tests { model.add_formula( crate::formula::parse_formula("Profit = Revenue - 1", "_Measure").unwrap(), ); - let measure = model.category_mut("_Measure").unwrap(); - measure.add_item("Profit"); - measure.set_item_display("Profit", PROFIT); let refused = |key: &CellKey, measure: &str| { let cell = RecordCell { record: RecordHash::of(key), @@ -5655,7 +5651,7 @@ mod non_item_key_tests { ("_Measure".into(), "Profit".into()), ]); let why = refused(&profit, "Profit").expect("a formula's result is refused"); - assert!(why.starts_with(PROFIT), "{why}"); + assert!(why.starts_with("Profit is computed by a formula"), "{why}"); } #[test] diff --git a/crates/improvise-protocol/src/resolve.rs b/crates/improvise-protocol/src/resolve.rs index 10dc4358..99760eaa 100644 --- a/crates/improvise-protocol/src/resolve.rs +++ b/crates/improvise-protocol/src/resolve.rs @@ -963,9 +963,7 @@ fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option> { "an aggregate is edited by drilling in — {}", not_yet("drill") ))]), - CellId::Derived { target } => { - Some(vec![Command::SetStatus(derived_decline(state, target))]) - } + CellId::Derived { target } => Some(vec![Command::SetStatus(derived_decline(target))]), // Both creating columns are records-only, so the guard above takes // them; declining rather than `None` keeps that a door, not a hole. CellId::Create(_) => Some(vec![Command::SetStatus(not_yet("creating a column"))]), @@ -1408,7 +1406,7 @@ fn reopened_at( pub(crate) fn declined_at(state: &ClientState, row: usize, col: usize) -> Option { match projected_at(state, row, col)?.id.as_ref()? { CellId::Aggregate(_) => Some(AGGREGATE_DECLINE.to_string()), - CellId::Derived { target } => Some(derived_decline(state, target)), + CellId::Derived { target } => Some(derived_decline(target)), CellId::NonItem { category, item } => Some(crate::non_item_refusal( state.categories.category_display(category), item, @@ -1445,8 +1443,7 @@ const AGGREGATE_DECLINE: &str = "Cannot write to an aggregate — it combines several records, so there is no one cell to write"; /// What both ends say of a formula's result: the host's `derived_refusal`. -fn derived_decline(state: &ClientState, target: &str) -> String { - let target = state.categories.item_display("_Measure", target); +fn derived_decline(target: &str) -> String { format!("{target} is computed by a formula — edit the formula, not the cell") } @@ -5044,7 +5041,6 @@ mod display_tests { const REGION: &str = "Sales territory of the whole company"; const NORTH: &str = "Northern reaches and the colder coasts"; - const TOTAL: &str = "Grand total of everything counted"; fn ev(code: KeyCode) -> KeyEvent { KeyEvent::new(code, KeyModifiers::NONE) @@ -5065,20 +5061,12 @@ mod display_tests { state.bootstrap(Snapshot { view: ViewState::default(), seed: vec![ - Projection::CategoryTree(CategoryTreeUpdate::Categories(vec![ - CategoryRow { - name: "Region".into(), - display: REGION.into(), - items: vec![shown("North", NORTH)], - axis: Axis::Row, - }, - CategoryRow { - name: "_Measure".into(), - display: "_Measure".into(), - items: vec![shown("Total", TOTAL)], - axis: Axis::Column, - }, - ])), + Projection::CategoryTree(CategoryTreeUpdate::Categories(vec![CategoryRow { + name: "Region".into(), + display: REGION.into(), + items: vec![shown("North", NORTH)], + axis: Axis::Row, + }])), Projection::TileBar(vec![TileEntry { category: "Region".into(), display: REGION.into(), @@ -5172,12 +5160,7 @@ mod display_tests { } #[test] - fn a_refused_cell_names_the_displays_it_shows() { - let derived = displayed(Some(CellId::Derived { - target: "Total".into(), - })); - let said = last_status(local(&derived, KeyCode::Char('i'))); - assert!(said.starts_with(TOTAL), "{said}"); + fn a_refused_cell_names_the_category_it_shows() { let stray = displayed(Some(CellId::NonItem { category: "Region".into(), item: "Mars".into(), From 040b7070e305c6a0f2c6bf342abe732720937a09 Mon Sep 17 00:00:00 2001 From: "Claude (adr007-slug)" Date: Fri, 9 Oct 2026 13:09:18 -0700 Subject: [PATCH 09/11] test(core): pin the zeros tie-break, widen the id-syntax and suffix properties, refresh the amendment's built-code note (improvise-cq1q.14) Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- .../adr/007-stable-identities-amendment-1.md | 11 +++-- context/test-evidence/core.md | 16 ++++++- crates/improvise-core/src/model/id.rs | 46 ++++++++++++++----- 3 files changed, 55 insertions(+), 18 deletions(-) diff --git a/context/adr/007-stable-identities-amendment-1.md b/context/adr/007-stable-identities-amendment-1.md index 940b8e38..63ebfbb7 100644 --- a/context/adr/007-stable-identities-amendment-1.md +++ b/context/adr/007-stable-identities-amendment-1.md @@ -178,10 +178,11 @@ different thing that generates the same slug: the merged file declares that id t and the load is refused naming both. Stable identity across copies of a file is not promised (improvise-8tk5). -**Built code.** `crates/improvise-core/src/model/id.rs` implements the accepted steps -(`context/code/core.md` §20). A follow-up must replace rather than strip, collapse -`_`, trim a trailing `_` and both ends' `-`, prepend `n`, make `claim` refuse what a -generated id could not be, and add the virtual views and `IdKind::Group`. +**Built code.** `crates/improvise-core/src/model/id.rs` implements this clause +(`context/code/core.md` §20): `slugify` follows the six steps, `is_id_syntax` is the +syntax, `claim` refuses what a generated id could not be, `VIRTUAL_IDS` holds the five +virtual ids, `IdKind::Group` exists, and `natural_cmp` and `display_suffixes` implement +natural order and clause 2's suffixes. No other code calls them yet. ### 5. Persistence @@ -310,7 +311,7 @@ therefore shows B as `east-1`, never as a bare `east`. - Scripts that name things by display break and must be rewritten over ids. - Two measures can share a display, and are shown as `Profit` and `Profit #2`. - Old files' `_Measure` declarations and hyphen- or digit-led names change on first save. -- `id.rs` and its tests need the follow-up in clause 4. +- `id.rs` implements clause 4; the code that sorts, shows and creates ids does not call it yet. ## Open questions diff --git a/context/test-evidence/core.md b/context/test-evidence/core.md index b25328ca..53eb3eb8 100644 --- a/context/test-evidence/core.md +++ b/context/test-evidence/core.md @@ -1955,12 +1955,24 @@ The property over arbitrary displays: a non-empty slug satisfies `generated_ids_are_distinct_well_formed_and_never_reserved`): no `_` collapse, no `n` prefix, keeping a trailing `_` or `-`. -### model::id::tests::id_syntax_agrees_with_its_regular_expression +### model::id::tests::id_syntax_agrees_with_an_independent_checker -`is_id_syntax` against the spelled-out rule over `[a-c0-9_-]{0,8}`. Rivals: +`is_id_syntax` against a hand-written scanner of `^[a-z](_?[a-z0-9-])*$` that +does not end in `-`, over `[a-cA-C0-9 _%é-]{0,8}`, so uppercase, space, symbols +and non-ASCII are reached. Dropping the alphabet check also fails it. Rivals: allowing `__`, allowing a leading digit and allowing a trailing `-` each fail here and in `claim_refuses_what_a_generated_id_could_not_be`. +### model::id::tests::natural_order_decides_the_zeros_tie_at_its_own_run_not_after_the_rest + +Rival: compare leading zeros only after every run compares equal in value. +`a-7c` then sorts after `a-007b`, and this test fails alone. + +### model::id::tests::suffixed_displays_are_distinct_when_literals_already_hold_suffixes + +Displays are drawn from `A`, `A #2`, `A #3` and `B`, so literal suffixes meet +generated ones. Rival: numbering without the skip fails it. + ### model::id::tests::claim_refuses_what_a_generated_id_could_not_be Rival: `claim` keeps the old alphabet-only check. `-x`, `x-`, `2024`, `a__b`, diff --git a/crates/improvise-core/src/model/id.rs b/crates/improvise-core/src/model/id.rs index dfdea0f1..2bb87b84 100644 --- a/crates/improvise-core/src/model/id.rs +++ b/crates/improvise-core/src/model/id.rs @@ -424,6 +424,13 @@ mod tests { assert_eq!(natural_cmp("x000", "x0"), Ordering::Greater); } + #[test] + fn natural_order_decides_the_zeros_tie_at_its_own_run_not_after_the_rest() { + assert_eq!(natural_cmp("a-7c", "a-007b"), Ordering::Less); + assert_eq!(natural_cmp("a-7b", "a-007a"), Ordering::Less); + assert_eq!(natural_cmp("a-007a", "a-7b"), Ordering::Greater); + } + #[test] fn natural_order_puts_the_id_that_runs_out_of_runs_first() { assert_eq!(natural_cmp("a", "a-1"), Ordering::Less); @@ -484,6 +491,23 @@ mod tests { assert!(display_suffixes(&[]).is_empty()); } + /// `^[a-z](_?[a-z0-9-])*$` and not ending in `-`, without the crate's own helpers. + fn regex_free_match(id: &str) -> bool { + let mut cs = id.chars().peekable(); + if !matches!(cs.next(), Some('a'..='z')) { + return false; + } + while cs.peek().is_some() { + if cs.peek() == Some(&'_') { + cs.next(); + } + if !matches!(cs.next(), Some('a'..='z' | '0'..='9' | '-')) { + return false; + } + } + !id.ends_with('-') + } + fn id_text() -> impl Strategy { "[a-c0-3_-]{0,7}" } @@ -497,11 +521,8 @@ mod tests { } #[test] - fn id_syntax_agrees_with_its_regular_expression(id in "[a-c0-9_-]{0,8}") { - let spelled = id.starts_with(|c: char| c.is_ascii_lowercase()) - && !id.ends_with(['_', '-']) - && !id.contains("__"); - prop_assert_eq!(is_id_syntax(&id), spelled); + fn id_syntax_agrees_with_an_independent_checker(id in "[a-cA-C0-9 _%é-]{0,8}") { + prop_assert_eq!(is_id_syntax(&id), regex_free_match(&id), "{}", id); } #[test] @@ -514,13 +535,16 @@ mod tests { } #[test] - fn suffixed_displays_are_distinct_when_the_literals_are_not_already_clashing( - ids in proptest::collection::hash_set("[a-c][0-9]{0,2}", 0..8), - display in "[AB]( #[2-4])?" + fn suffixed_displays_are_distinct_when_literals_already_hold_suffixes( + shared in proptest::collection::vec(0usize..4, 0..8) ) { - let mut ids: Vec = ids.into_iter().collect(); - ids.sort_by(|a, b| natural_cmp(a, b)); - let items: Vec<(&str, &str)> = ids.iter().map(|i| (i.as_str(), display.as_str())).collect(); + let displays = ["A", "A #2", "A #3", "B"]; + let ids: Vec = (0..shared.len()).map(|n| format!("x-{n}")).collect(); + let items: Vec<(&str, &str)> = ids + .iter() + .zip(&shared) + .map(|(i, &d)| (i.as_str(), displays[d])) + .collect(); let shown = display_suffixes(&items); let distinct: HashSet<&String> = shown.iter().collect(); prop_assert_eq!(distinct.len(), shown.len()); From 369d1e12c0f7dc27d2cda8d97203d862d1c64570 Mon Sep 17 00:00:00 2001 From: "Claude (adr007-2c)" Date: Fri, 9 Oct 2026 13:30:42 -0700 Subject: [PATCH 10/11] test(display): pin the browser's declined_at arm and the removal operands to keys (improvise-cq1q.13) Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- context/test-evidence/core.md | 13 +++++++++++ .../src/command/cmd/panel.rs | 4 ++++ crates/improvise-protocol/src/resolve.rs | 22 ++++++++++++++++++- 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/context/test-evidence/core.md b/context/test-evidence/core.md index 6a446212..68d076d6 100644 --- a/context/test-evidence/core.md +++ b/context/test-evidence/core.md @@ -2187,6 +2187,11 @@ axis status passes `cat_name` (fails); the filter status passes the item key (fails); the item-add mode is built from the display (fails on the `category: "Type"` assertion). +Review follow-up: the same test now also pins the operands. Rivals: the host +`RemoveCategory` and `SetPageSelection` given the display fail on the +`RemoveCategory("Type")` and `item: "Food"` assertions. The client twin fails the +same way on `RemoveCategory`. + ### command::cmd::tile::tests::a_tile_status_names_the_display_not_the_key Rival: the status formats `name`. It fails on both the present display and the @@ -2208,3 +2213,11 @@ above stay green and this one fails. The wire's own refusal in `Grids::unwritable`. Rival: the category key; fails. The formula arm (once a third copy of the derived wording, now `derived_refusal`) is pinned to its wording so the consolidation cannot change what it says. + +### resolve::display_tests::the_declines_an_advance_and_a_commit_reach_name_the_category_it_shows + +From the Opus review of #339: the browser's `declined_at` non-item arm could pass the +key with the whole suite green. It is reached by `reopened_at` (an advance), +`refused_here` (a commit) and `settle_declined_editor`. Rival: the arm passes +`category`; the test fails on all three assertions, and the display is 36 characters +against the key "Region". diff --git a/crates/improvise-command/src/command/cmd/panel.rs b/crates/improvise-command/src/command/cmd/panel.rs index 83855380..beaa53a3 100644 --- a/crates/improvise-command/src/command/cmd/panel.rs +++ b/crates/improvise-command/src/command/cmd/panel.rs @@ -208,6 +208,8 @@ mod tests { assert_eq!(ctx.cat_at_cursor().as_deref(), Some("Type"), "setup"); assert!(said(&CycleAxisAtCursor.execute(&ctx)).contains(&format!("{TYPE} → "))); assert!(said(&DeleteCategoryAtCursor.execute(&ctx)).contains(&format!("'{TYPE}'"))); + let removal = format!("{:?}", DeleteCategoryAtCursor.execute(&ctx)); + assert!(removal.contains("RemoveCategory(\"Type\")"), "{removal}"); let prompt = format!("{:?}", OpenItemAddAtCursor.execute(&ctx)); assert!( prompt.contains(&format!("add item to {TYPE}: ")), @@ -219,6 +221,8 @@ mod tests { ); ctx.cat_panel_cursor = 4; + let filtering = format!("{:?}", FilterToItem.execute(&ctx)); + assert!(filtering.contains("item: \"Food\""), "keys: {filtering}"); let filter = said(&FilterToItem.execute(&ctx)); assert!( filter.contains(&format!("Filter: {TYPE} = {FOOD}")), diff --git a/crates/improvise-protocol/src/resolve.rs b/crates/improvise-protocol/src/resolve.rs index 99760eaa..808292b7 100644 --- a/crates/improvise-protocol/src/resolve.rs +++ b/crates/improvise-protocol/src/resolve.rs @@ -5106,7 +5106,14 @@ mod display_tests { state.view.cat_panel_cursor = 0; let cycled = last_status(local(&state, KeyCode::Char(' '))); assert_eq!(cycled, format!("{REGION} → Col")); - let deleted = last_status(delete_at_cat_cursor(&state).unwrap()); + let removal = delete_at_cat_cursor(&state).unwrap(); + assert!( + removal.iter().any(|c| matches!(c, Command::Model { + cmd: ModelCommand::RemoveCategory(name), .. + } if name == "Region")), + "the removal names the key: {removal:?}" + ); + let deleted = last_status(removal); assert_eq!(deleted, format!("Deleted category '{REGION}'")); state.view.cat_panel_cursor = 1; @@ -5159,6 +5166,19 @@ mod display_tests { assert_eq!(minibuf.prompt, format!("add item to {REGION}: ")); } + #[test] + fn the_declines_an_advance_and_a_commit_reach_name_the_category_it_shows() { + let stray = displayed(Some(CellId::NonItem { + category: "Region".into(), + item: "Mars".into(), + })); + let want = crate::non_item_refusal(REGION, "Mars"); + assert_eq!(declined_at(&stray, 0, 0), Some(want.clone())); + assert_eq!(refused_here(&stray, "commit-cell-edit"), Some(want.clone())); + let advanced = reopened_at(&stray, 0, 0, AppMode::Normal, false); + assert_eq!(advanced, vec![Command::SetStatus(want)]); + } + #[test] fn a_refused_cell_names_the_category_it_shows() { let stray = displayed(Some(CellId::NonItem { From 52453ca785f2e570063bdbe6660a53a32de63d3a Mon Sep 17 00:00:00 2001 From: "Claude (adr009-zset-amend)" Date: Sat, 10 Oct 2026 11:14:46 -0700 Subject: [PATCH 11/11] docs(adr): ADR 009 is accepted; Z-set weights are checked i64 and the group is partial (improvise-felo.4) Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- context/adr/009-incremental-evaluation.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/context/adr/009-incremental-evaluation.md b/context/adr/009-incremental-evaluation.md index 68135713..f0506249 100644 --- a/context/adr/009-incremental-evaluation.md +++ b/context/adr/009-incremental-evaluation.md @@ -1,6 +1,6 @@ # ADR 009 — Incremental evaluation: an in-house DBSP core, plugged in behind a trait -**Status:** proposed. · +**Status:** accepted. · **Ticket:** improvise-felo.4 (the record of this ADR; implementation goes to its own epic) · **Relates to:** ADR 001 (the basis a write claims), ADR 002 (surrogates on the wire), @@ -29,7 +29,11 @@ mutation, it says so and names the incremental path that normally applies. A problem-agnostic workspace crate with no dependencies beyond `std` (so it is wasm32-clean and serde-free). It provides: -- `ZSet`, a map from `K: Ord` to a non-zero `i64` weight, which is an abelian group; +- `ZSet`, a map from `K: Ord` to a non-zero `i64` weight. Weight arithmetic is checked + and panics on overflow, in debug and in release. The group laws are exact wherever no + weight, intermediate ones included, leaves `i64`, so the structure is an abelian group + that is partial at the `i64` boundary. Weights are validated where deltas enter from + the wire (clause 9), so a hostile frame is rejected and never reaches the panic; - a synchronous, single-threaded `Circuit` per process, with `delay`, `integrate`, `differentiate`, `map`, `filter`, `flat_map`, `index_by`, the bilinear delta join `join_indexed`, `distinct` and `aggregate`;