From 49ea84ca5fe7bbc861a7026c4f66bfd9e848ce5f Mon Sep 17 00:00:00 2001 From: "Claude (felo13)" Date: Fri, 2 Oct 2026 19:18:47 -0700 Subject: [PATCH] fix(docs): register the layout evidence pages; test the two gaps the review found (improvise-felo.13) Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- context/test-evidence/layout-cost.md | 22 ++++++++++++++++ crates/improvise-command/src/ui/app.rs | 26 +++++++++++++++++++ crates/improvise-core/src/view/layout.rs | 32 ++++++++++++++++++++++++ scripts/build-doc-site.mjs | 2 ++ 4 files changed, 82 insertions(+) diff --git a/context/test-evidence/layout-cost.md b/context/test-evidence/layout-cost.md index ddea2b06..e51fad20 100644 --- a/context/test-evidence/layout-cost.md +++ b/context/test-evidence/layout-cost.md @@ -101,3 +101,25 @@ what `grid_body` builds for the same window, over the whole grid, an interior window, the last row and the empty window. Rival (`grid_window` reading column `ci + 1`): fails at `window 0..2 x 0..10`. Widths stay the **whole-grid** maximum, because the summary is the same walk `grid_body` did; nothing the user sees moves. + +### a_frozen_records_snapshot_replaces_the_live_row_index + +`view::layout::lookup_cost_tests`. Three records are frozen, two are then cleared, +and `with_frozen_records` over the snapshot must draw 3 rows (the control: with no +snapshot it draws 1). A drill held open while another client removes records is the +production analogue. Rival (deleting `layout.reindex()` after the snapshot swap): +fails `left: 1, right: 3`; the whole workspace passed it before this test. + +### a_drill_snapshot_replaced_alone_rebuilds_the_layout + +`ui::app::tests`. `StartDrill` is `ViewOnly`, so a batch of it alone has no `Model` +effect to mark the layout stale; the drill pointer in `LayoutBasis` is the only +thing that rebuilds it. The test starts one snapshot, then a second: the first +step alone passes even with a constant pointer (drill absent to present changes the +basis), which is why the second step is the test. Rival (pointer replaced by a +constant): fails `no rebuild for a second drill snapshot`. + +### Observation: the TUI frame + +`tui_frame_grid_of_whole` is still 47-53 ms at 19,683 rows after this change: the +draw path projects the whole grid, linear and out of scope for this ticket. diff --git a/crates/improvise-command/src/ui/app.rs b/crates/improvise-command/src/ui/app.rs index 0419f863..e28d8c87 100644 --- a/crates/improvise-command/src/ui/app.rs +++ b/crates/improvise-command/src/ui/app.rs @@ -1663,6 +1663,32 @@ mod tests { app } + /// improvise-felo.13: `StartDrill` is `ViewOnly`, so a batch of it alone moves + /// the layout with no `Model` effect to mark it stale. Evidence: layout-cost.md. + #[test] + fn a_drill_snapshot_replaced_alone_rebuilds_the_layout() { + let mut app = records_app_for_drill(); + assert_eq!(app.layout.row_count(), 3); + let rows: Vec<_> = app.layout.records.as_ref().unwrap().to_vec(); + let start = |app: &mut App, n: usize| { + let snapshot = rows.iter().take(n).cloned().collect(); + app.apply_effects(vec![Box::new(crate::ui::effect::StartDrill(snapshot))]); + }; + start(&mut app, 1); + assert_eq!( + app.layout.row_count(), + 1, + "the layout draws the frozen snapshot" + ); + let builds = app.layout_builds; + start(&mut app, 2); + assert!( + app.layout_builds > builds, + "no rebuild for a second drill snapshot" + ); + assert_eq!(app.layout.row_count(), 2); + } + /// The opening basis may be read off `App::layout` only when no drill froze /// it. Evidence: `context/test-evidence/command.md`. #[test] diff --git a/crates/improvise-core/src/view/layout.rs b/crates/improvise-core/src/view/layout.rs index 269745b5..5b4c93fa 100644 --- a/crates/improvise-core/src/view/layout.rs +++ b/crates/improvise-core/src/view/layout.rs @@ -3133,6 +3133,38 @@ mod lookup_cost_tests { } } + /// improvise-felo.13: a drill held open while records are removed elsewhere. + /// Evidence: layout-cost.md. + #[test] + fn a_frozen_records_snapshot_replaces_the_live_row_index() { + use crate::model::cell::{CellKey, CellValue}; + let mut wb = Workbook::new("T"); + wb.add_category("Region").unwrap(); + wb.add_item("_Measure", "Revenue").unwrap(); + let cell = |i: usize| { + CellKey::new(vec![ + ("Region".into(), format!("r{i}")), + ("_Measure".into(), "Revenue".into()), + ]) + }; + for i in 0..3 { + wb.add_item("Region", format!("r{i}")).unwrap(); + wb.model.set_cell(cell(i), CellValue::Number(1.0)); + } + let v = wb.active_view_mut(); + v.set_axis("_Index", Axis::Row); + v.set_axis("_Dim", Axis::Column); + let frozen = GridLayout::new(&wb.model, wb.active_view()).records; + wb.model.clear_cell(&cell(1)); + wb.model.clear_cell(&cell(2)); + let live = GridLayout::with_frozen_records(&wb.model, wb.active_view(), None); + assert_eq!(live.row_count(), 1, "the control: one record is live"); + let held = GridLayout::with_frozen_records(&wb.model, wb.active_view(), frozen); + assert_eq!(held.row_count(), 3); + assert_eq!(held.row_tuple(2), Some(&["2".to_string()][..])); + assert!(held.cell_key(2, 0).is_some()); + } + fn data_items_entry(items: &[super::AxisEntry], n: usize) -> Option<&super::AxisEntry> { items .iter() diff --git a/scripts/build-doc-site.mjs b/scripts/build-doc-site.mjs index f9bab7b3..53cee6eb 100644 --- a/scripts/build-doc-site.mjs +++ b/scripts/build-doc-site.mjs @@ -81,6 +81,8 @@ const SECTIONS = [ ["context/test-evidence/cli.md", "test-evidence/cli", "The headless CLI"], ["context/test-evidence/gp.md", "test-evidence/gp", "gp-tree and improvise-gp"], ["context/test-evidence/tooling.md", "test-evidence/tooling", "The repository's own gates"], + ["context/test-evidence/layout-cost.md", "test-evidence/layout-cost", "Layout and keypress cost"], + ["context/test-evidence/scale-bisect.md", "test-evidence/scale-bisect", "Scale bisect"], ], }, {