From e5721b9d94d1ac774032b07cf6a0729f72ca9d91 Mon Sep 17 00:00:00 2001 From: Edward Langley Date: Tue, 22 Sep 2026 22:37:25 -0700 Subject: [PATCH] improvise-lxg0/w1bu: a fresh session has no history; records kinds checked per column fresh_view points the new slice at the document's view instead of folding a SwitchView from ViewState::default(), which recorded a visit to "Default" that never happened. The records-grid enumeration now checks each cell's CellId against its column's RecordCol rather than only that it is Some. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- crates/improvise-command/src/ui/project.rs | 89 +++++++++++++++++++--- 1 file changed, 77 insertions(+), 12 deletions(-) diff --git a/crates/improvise-command/src/ui/project.rs b/crates/improvise-command/src/ui/project.rs index ff23ca77..77719266 100644 --- a/crates/improvise-command/src/ui/project.rs +++ b/crates/improvise-command/src/ui/project.rs @@ -1119,12 +1119,11 @@ fn view_names(app: &App) -> std::collections::BTreeSet { /// The view slice a newly-connected client starts from: empty, but pointed at the /// document's active view, because that is what keys its cursor map (gotcha 29). fn fresh_view(app: &App) -> ViewState { - let mut view = ViewState::default(); - reduce_view( - &mut view, - &Command::SwitchView(app.model_state.workbook.active_view.clone()), - ); - view + // Pointed, not switched: a fold would record a visit to `Default` (improvise-lxg0). + ViewState { + active_view: app.model_state.workbook.active_view.clone(), + ..ViewState::default() + } } #[cfg(test)] @@ -1569,6 +1568,56 @@ mod tests { app } + /// A document with the views `names`, opening on `active`. + fn opening_on(names: &[&str], active: &str) -> Workbook { + let mut wb = Workbook::new("Fresh"); + for name in names.iter().filter(|n| **n != "Default") { + wb.create_view(name.to_string()); + } + wb.active_view = active.to_string(); + if !names.contains(&"Default") { + wb.delete_view("Default") + .expect("setup: drop the Default view"); + } + wb + } + + /// improvise-lxg0: a new session has visited nothing, and `<` must not + /// take it anywhere. Evidence: `context/test-evidence/command.md`. + #[test] + fn a_fresh_session_has_no_history_to_step_back_through() { + for names in [&["Default", "Second"][..], &["Main", "Second"][..]] { + let app = App::new(opening_on(names, "Second"), None); + let mut sessions = Sessions::default(); + let (_, snapshot) = sessions.connect(&app, 10, 10); + assert_eq!(snapshot.view.active_view, "Second", "setup: {names:?}"); + assert_eq!(snapshot.view.view_back_stack, vec![], "{names:?}"); + + let mut client = snapshot.view.clone(); + reduce_view(&mut client, &Command::ViewBack); + assert_eq!( + client.active_view, "Second", + "`<` moved a fresh {names:?} client" + ); + } + } + + /// improvise-lxg0: a document swap reseeds through the same door. + #[test] + fn a_rebased_session_has_no_history_to_step_back_through() { + for names in [&["Default", "Second"][..], &["Main", "Second"][..]] { + let mut app = App::new(opening_on(&["Default", "Other"], "Default"), None); + let mut sessions = Sessions::default(); + sessions.connect(&app, 10, 10); + app.install_workbook(opening_on(names, "Second")); + let snapshots = sessions.rebase(&app); + assert_eq!(snapshots.len(), 1, "setup: one session rebased"); + let view = &snapshots[0].1.view; + assert_eq!(view.active_view, "Second", "setup: {names:?}"); + assert_eq!(view.view_back_stack, vec![], "{names:?}"); + } + } + /// improvise-el9h: the mirror must settle against the grid it ends up /// holding. Evidence: `context/test-evidence/command.md`. #[test] @@ -2679,7 +2728,7 @@ mod tests { } for name in ["Default", "Second"] { if name != "Default" { - wb.create_view(name); + wb.create_view(name.to_string()); } let v = &mut wb.views[name]; v.set_axis("Region", Axis::Row); @@ -2783,7 +2832,7 @@ mod tests { ); for name in ["Default", "Second", "Third"] { if name != "Default" { - wb.create_view(name); + wb.create_view(name.to_string()); } let v = &mut wb.views[name]; v.set_axis("Region", Axis::Row); @@ -5163,16 +5212,32 @@ mod cell_identity { Some(CellId::Coordinate), "a drawn coordinate is named" ); - let nameless: Vec<(usize, usize)> = (0..app.layout.row_count()) + // Each cell's kind against its column's, not merely "some kind" (improvise-w1bu). + let misnamed: Vec<(usize, usize, Option)> = (0..app.layout.row_count()) .flat_map(|r| (0..app.layout.col_count()).map(move |c| (r, c))) - .filter(|&(r, c)| grid.cell(r, c).is_some_and(|cell| cell.id.is_none())) + .map(|(r, c)| (r, c, grid.cell(r, c).and_then(|cell| cell.id.clone()))) + .filter(|(_, c, id)| !names_its_column(app.layout.record_col(*c), id.as_ref())) .collect(); assert!( - nameless.is_empty(), - "every records cell is something: {nameless:?}" + misnamed.is_empty(), + "a records cell of the wrong kind: {misnamed:?}" ); } + /// Whether `id` is the kind a records column of `col`'s kind projects. + fn names_its_column(col: Option<&RecordCol>, id: Option<&CellId>) -> bool { + match (col, id) { + (Some(RecordCol::Category(_)), Some(CellId::Coordinate)) => true, + (Some(RecordCol::Measure(m)), Some(CellId::Record(cell))) => { + cell.measure.as_deref() == Some(m.as_str()) + } + (Some(RecordCol::Measure(_)), Some(CellId::Derived { .. })) => true, + (Some(RecordCol::NewMeasure), Some(CellId::Create(CreateKind::Measure))) => true, + (Some(RecordCol::NewCategory), Some(CellId::Create(CreateKind::Category))) => true, + _ => false, + } + } + /// The version counter is keyed by the surrogate the client holds, so one /// identity answers ADR 002's question and ADR 001's. #[test]