From e0bd0be3d5946188a56b93891bb718187e05f099 Mon Sep 17 00:00:00 2001 From: "Claude (adr007-2c)" Date: Fri, 9 Oct 2026 12:48:50 -0700 Subject: [PATCH 1/6] 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 bf7e999edcdbf185bdee69a22d6d8bef4e9f4de4 Mon Sep 17 00:00:00 2001 From: "Claude (adr007-2c)" Date: Fri, 9 Oct 2026 12:49:12 -0700 Subject: [PATCH 2/6] 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 3/6] 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 bb7fe88547e9733cf954a9af1e0e6d9d23c24001 Mon Sep 17 00:00:00 2001 From: "Claude (adr007-2c)" Date: Fri, 9 Oct 2026 12:55:41 -0700 Subject: [PATCH 4/6] =?UTF-8?q?docs(context):=20core.md=20=C2=A721=20third?= =?UTF-8?q?=20slice,=20test=20evidence,=20and=20two=20comment=20runs=20sho?= =?UTF-8?q?rtened=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 5/6] 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 369d1e12c0f7dc27d2cda8d97203d862d1c64570 Mon Sep 17 00:00:00 2001 From: "Claude (adr007-2c)" Date: Fri, 9 Oct 2026 13:30:42 -0700 Subject: [PATCH 6/6] 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 {