From 14cef311dd6cce1bd60d0a4b57b4a88cb60c4756 Mon Sep 17 00:00:00 2001 From: "Claude (felo13)" Date: Fri, 2 Oct 2026 19:01:46 -0700 Subject: [PATCH] docs(evidence): layout cost evidence and measurements; fmt the merged bench (improvise-felo.13) Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- benches/scale_bisect.rs | 127 ++++++++++++++++---- context/test-evidence/layout-cost.md | 103 ++++++++++++++++ crates/improvise-command/src/ui/app.rs | 39 ++++-- crates/improvise-command/src/ui/project.rs | 14 ++- crates/improvise-command/src/ui/viewport.rs | 13 +- crates/improvise-core/src/model/cell.rs | 62 +++++++--- crates/improvise-core/src/view/layout.rs | 49 ++++++-- 7 files changed, 336 insertions(+), 71 deletions(-) create mode 100644 context/test-evidence/layout-cost.md diff --git a/benches/scale_bisect.rs b/benches/scale_bisect.rs index 9b559514..cc58e0a7 100644 --- a/benches/scale_bisect.rs +++ b/benches/scale_bisect.rs @@ -79,7 +79,11 @@ fn document(c: &Cfg) -> (Workbook, Vec) { } let view = wb.active_view_mut(); for ci in 0..c.cats { - let axis = if c.mode == "records" || c.mode == "none" { Axis::None } else { Axis::Row }; + let axis = if c.mode == "records" || c.mode == "none" { + Axis::None + } else { + Axis::Row + }; view.set_axis(&format!("C{ci}"), axis); } if c.mode == "records" { @@ -103,7 +107,11 @@ fn rss_mb() -> f64 { .args(["-o", "rss=", "-p", &std::process::id().to_string()]) .output() .expect("ps"); - String::from_utf8_lossy(&out.stdout).trim().parse::().unwrap_or(0.0) / 1024.0 + String::from_utf8_lossy(&out.stdout) + .trim() + .parse::() + .unwrap_or(0.0) + / 1024.0 } /// Sample `f` up to `reps` times, stopping once `budget_ms` is spent (one sample minimum). @@ -132,7 +140,10 @@ fn sample(name: &str, reps: usize, budget_ms: f64, mut f: impl FnMut(usize)) { fn once(name: &str, f: impl FnOnce() -> T) -> T { let t = Instant::now(); let v = f(); - println!("{name} min={0:.3} med={0:.3} max={0:.3} reps=1", t.elapsed().as_secs_f64() * 1000.0); + println!( + "{name} min={0:.3} med={0:.3} max={0:.3} reps=1", + t.elapsed().as_secs_f64() * 1000.0 + ); std::io::stdout().flush().ok(); v } @@ -144,7 +155,10 @@ fn key(code: KeyCode) -> KeyEvent { fn main() { let args: HashMap = std::env::args() .skip(1) - .filter_map(|a| a.split_once('=').map(|(k, v)| (k.to_string(), v.to_string()))) + .filter_map(|a| { + a.split_once('=') + .map(|(k, v)| (k.to_string(), v.to_string())) + }) .collect(); let c = Cfg { k: arg(&args, "k", "1000").parse().expect("k"), @@ -153,19 +167,35 @@ fn main() { prune: arg(&args, "prune", "1") == "1", mode: arg(&args, "mode", "grid"), groups: arg(&args, "groups", "0") == "1", - ops: arg(&args, "ops", "open,key,frame,window,setcell,host,reshape,io"), + ops: arg( + &args, + "ops", + "open,key,frame,window,setcell,host,reshape,io", + ), }; let rows_cap: f64 = arg(&args, "rows_cap", "30000000").parse().expect("cap"); let product = (c.k as f64).powi(c.cats as i32); if c.mode == "grid" && product > rows_cap { - println!("REFUSED product_rows={product:.3e} (> {rows_cap:.0}: layout would materialize it)"); + println!( + "REFUSED product_rows={product:.3e} (> {rows_cap:.0}: layout would materialize it)" + ); return; } let want = |o: &str| c.ops.split(',').any(|x| x == o); let (wb, recs) = document(&c); - println!("cells={} records={} base_rss_mb={:.0}", wb.model.data.iter_cells().count(), recs.len(), rss_mb()); + println!( + "cells={} records={} base_rss_mb={:.0}", + wb.model.data.iter_cells().count(), + recs.len(), + rss_mb() + ); let mut app = once("open", || App::new(wb, None)); - println!("rows={} cols={} rss_after_open_mb={:.0}", app.layout.row_count(), app.layout.col_count(), rss_mb()); + println!( + "rows={} cols={} rss_after_open_mb={:.0}", + app.layout.row_count(), + app.layout.col_count(), + rss_mb() + ); app.term_width = 200; app.term_height = 50; let name = app.view_name().to_string(); @@ -176,7 +206,12 @@ fn main() { }); } if want("frame") { - let whole = |app: &App| Viewport { row_offset: 0, col_offset: 0, rows: app.layout.row_count(), cols: app.layout.col_count() }; + let whole = |app: &App| Viewport { + row_offset: 0, + col_offset: 0, + rows: app.layout.row_count(), + cols: app.layout.col_count(), + }; sample("tui_frame_grid_of_whole", 3, 8000.0, |_| { std::hint::black_box(project::grid_of(&app, whole(&app))); }); @@ -185,9 +220,14 @@ fn main() { sample("tui_render_gridwidget_buffer", 5, 4000.0, |_| { let mut buf = ratatui::buffer::Buffer::empty(area); let w = improvise::ui::grid::GridWidget::new( - &cache, app.cursor(), &app.view_state.active_view, &app.layout, - &app.view_state.mode, &app.view_state.search_query, - &app.view_state.buffers, app.drill_state.as_ref(), + &cache, + app.cursor(), + &app.view_state.active_view, + &app.layout, + &app.view_state.mode, + &app.view_state.search_query, + &app.view_state.buffers, + app.drill_state.as_ref(), ); ratatui::widgets::Widget::render(w, area, &mut buf); std::hint::black_box(buf); @@ -196,7 +236,12 @@ fn main() { if want("window") { let mid = app.layout.row_count() / 2; for (label, off) in [("top", 0), ("mid", mid)] { - let vp = Viewport { row_offset: off, col_offset: 0, rows: 45, cols: 12 }; + let vp = Viewport { + row_offset: off, + col_offset: 0, + rows: 45, + cols: 12, + }; sample(&format!("browser_grid_of_{label}"), 5, 6000.0, |_| { std::hint::black_box(project::grid_of(&app, vp)); }); @@ -205,29 +250,49 @@ fn main() { if want("setcell") && !recs.is_empty() { let existing = recs[recs.len() / 2].clone().with("_Measure", "Rev"); sample("setcell_existing", 5, 6000.0, |r| { - app.apply_effects(vec![Box::new(effect::SetCell(existing.clone(), CellValue::Number(10.0 + r as f64)))]); + app.apply_effects(vec![Box::new(effect::SetCell( + existing.clone(), + CellValue::Number(10.0 + r as f64), + ))]); }); let mut s = 99u64; sample("setcell_new_record", 5, 6000.0, |_| { - let mut coords: Vec<(String, String)> = (0..c.cats).map(|ci| (format!("C{ci}"), format!("i{}", lcg(&mut s, c.k)))).collect(); + let mut coords: Vec<(String, String)> = (0..c.cats) + .map(|ci| (format!("C{ci}"), format!("i{}", lcg(&mut s, c.k)))) + .collect(); coords.push(("_Measure".into(), "Rev".into())); - app.apply_effects(vec![Box::new(effect::SetCell(CellKey::new(coords), CellValue::Number(1.0)))]); + app.apply_effects(vec![Box::new(effect::SetCell( + CellKey::new(coords), + CellValue::Number(1.0), + ))]); }); } if want("reshape") && c.mode == "grid" { let v = || ViewTarget::Active; sample("reshape_hide_item", 3, 6000.0, |r| { - app.apply_effects(vec![Box::new(effect::HideItem { view: v(), category: "C1".into(), item: format!("i{r}") })]); + app.apply_effects(vec![Box::new(effect::HideItem { + view: v(), + category: "C1".into(), + item: format!("i{r}"), + })]); }); if c.groups { sample("reshape_toggle_group", 3, 6000.0, |_| { - app.apply_effects(vec![Box::new(effect::ToggleGroup { view: v(), category: "C0".into(), group: "G0".into() })]); + app.apply_effects(vec![Box::new(effect::ToggleGroup { + view: v(), + category: "C0".into(), + group: "G0".into(), + })]); }); } let last = format!("C{}", c.cats - 1); sample("reshape_set_axis", 4, 6000.0, |r| { let axis = if r % 2 == 0 { Axis::Page } else { Axis::Row }; - app.apply_effects(vec![Box::new(effect::SetAxis { view: v(), category: last.clone(), axis })]); + app.apply_effects(vec![Box::new(effect::SetAxis { + view: v(), + category: last.clone(), + axis, + })]); }); sample("reshape_toggle_prune", 2, 6000.0, |_| { app.apply_effects(vec![Box::new(effect::TogglePruneEmpty(v()))]); @@ -249,15 +314,31 @@ fn main() { sample("json_encode_snapshot", 3, 4000.0, |_| { std::hint::black_box(serde_json::to_vec(&snap.msg).expect("json")); }); - println!("snapshot_kb={:.1}", serde_json::to_vec(&snap.msg).expect("json").len() as f64 / 1024.0); + println!( + "snapshot_kb={:.1}", + serde_json::to_vec(&snap.msg).expect("json").len() as f64 / 1024.0 + ); let rec = recs[recs.len() / 2].clone(); - let cell = RecordCell { record: RecordHash::of(&rec), measure: Some("Rev".into()) }; + let cell = RecordCell { + record: RecordHash::of(&rec), + measure: Some("Rev".into()), + }; let mut enc: Vec<(f64, usize)> = Vec::new(); sample("host_handle_setcell_and_project", 4, 8000.0, |r| { - let wire = caught_up(host.app(), ModelCommand::SetCell { view: name.clone(), cell: cell.clone(), value: CellValue::Number(50.0 + r as f64) }); + let wire = caught_up( + host.app(), + ModelCommand::SetCell { + view: name.clone(), + cell: cell.clone(), + value: CellValue::Number(50.0 + r as f64), + }, + ); let out = host.handle(id, ClientFrame::command(wire)); let t = Instant::now(); - let bytes: usize = out.iter().map(|a| serde_json::to_vec(&a.msg).expect("json").len()).sum(); + let bytes: usize = out + .iter() + .map(|a| serde_json::to_vec(&a.msg).expect("json").len()) + .sum(); enc.push((t.elapsed().as_secs_f64() * 1000.0, bytes)); }); println!("json_encode_setcell_updates ms={:?}", enc); diff --git a/context/test-evidence/layout-cost.md b/context/test-evidence/layout-cost.md new file mode 100644 index 00000000..ddea2b06 --- /dev/null +++ b/context/test-evidence/layout-cost.md @@ -0,0 +1,103 @@ +# Layout and keypress cost (improvise-felo.13) + +Measured on an **Apple M4 Max, 64 GB**, macOS, release build, with +`benches/scale_bisect.rs` (merged from `bench/scale-bisect`, `13918bb9`), one +configuration per process, 3 categories of K items on Row, N = 100,000 capped at +K^3/2 records, prune off unless stated. Baseline is this branch with only the +tests added; "after" is the branch head. `key` is `App::dispatch_key(Down)`; +"cold" is the first key after open or after an edit, which pays the one +whole-grid measurement; "warm" is every key after it. + +## Before and after + +| rows | before key | after key warm | after key cold | before browser window | after browser window | SetCell before / after | +|---|---|---|---|---|---|---| +| 729 (K=9) | 5.5 | 0.00 | 1.4 | 5.0 | 0.13 | 0.9 / 0.9 | +| 4,913 (K=17) | 108 | 0.00 | 6.0 | 105 | 0.12 | 5.8 / 5.9 | +| 5,832 (K=18) | | 0.00 | 7.7 | | 0.12 | 6.8 | +| 6,859 (K=19) | | 0.00 | 8.8 | | 0.12 | 8.6 | +| 19,683 (K=27) | 1,271 | 0.00 | 24.5 | 1,259 | 0.14 | 23.0 / 22.7 | +| 64,000 (K=40) | 10,771 | 0.00 | 84 | 10,746 | 0.20 | 79 / 80 | + +Prune on: K=17 (2,456 rows) key 264 ms -> 0.00 warm, 3.6 cold; prune toggle +172 -> 8.8 ms; K=27 prune toggle 2,189 -> 41.5 ms. Records mode (3 x 1000, 14,500 +records): key 1,004 ms -> 0.00 warm, 2.8 cold; **SetCell 1,511 ms -> 1,512 ms, +unchanged** (not this ticket's cost). + +Boundaries, warm key and browser window: inside 8 ms at every size measured (to +216,000 rows, 303 ms cold). Cold key: inside 8 ms to K=18 (5,832 rows), over 8 ms at +K=19. Before: inside 8 ms to 729 rows, over 100 ms at 4,913, over 1 s at 19,683. +The remaining cold cost is linear (1.2-1.4 us per row) and is the whole-grid width +and total pass; the warm path does not repeat it. + +## What the ticket's reading got right and what it did not predict + +The O(rows^2) in `cell_key` was real and is one of three causes. Two more sat +under it. First, `compute_col_widths` ran on **every** keypress, through +`cmd_context`, and `dispatch_key` and `apply_effects` each rebuilt the layout +whether or not anything had moved. Second, once the index was O(1) the width pass +was still O(K^2) per cell: with `_Index`/`_Dim` on `Axis::None` the aggregation is +not `Exact`, so each cell went through `DataStore::matching_values`, which +narrows by the smallest index set (about N/K candidates) and probes the rest. A +partial as wide as the widest stored cell can match only one cell, so +`exact_positions` probes it directly. That second cause is why K=40's cold key +stayed at 8 s after the first two fixes. + +### a_cursor_key_rebuilds_neither_the_layout_nor_the_widths + +`ui::app::tests`. Two keys after a warm-up leave `layout_builds` and +`width_passes` where they were. Failed before the fix on the layout count (a +cursor key rebuilt it). Rival 1 (`refresh_layout` always rebuilding): fails on +`layout rebuilt on a cursor key`. The test seats the cursor at row 2, so "the +keys moved the cursor" is asserted and a key that does nothing cannot pass. + +### an_edit_rebuilds_and_remeasures_so_the_widths_follow_the_value + +The control for the test above, and the guard on invalidation. A `SetCell` of a +long text must rebuild the layout and remeasure, and the cached widths must equal +a fresh `compute_col_widths`. Rival 2 (`rebuild_layout` not dropping the cached +geometry): fails on `an edit must remeasure`. It also asserts the widths changed, +so a fixture where the edit does not widen a column cannot pass. + +### switching_view_rebuilds_though_the_effect_is_view_only + +`SwitchView` is `ViewOnly` (gotcha 3), so skipping the rebuild by `EffectKind` +alone would leave the old view's layout. The layout records the view it drew and +the drill snapshot it froze, and rebuilds when either differs. + +### an_axis_lookup_does_work_independent_of_the_row_index + +`view::layout::lookup_cost_tests`. Counts the items `data_items` yields during +`cell_key`, `row_tuple`, `row_label` and `column_key`; row 399 of 400 must cost no +more than row 0 plus two. Failed before the fix: `cell_key: row 399 cost 402 steps +against 3 at row 0`. Rival (`cell_key` going back to `data_items(..).nth(row)`): +`cost 400 steps against 1 at row 0`. Limit: the counter lives in `data_items`, so +a different linear walk written without it is not seen; the next test is what +holds the index to the walk's answers. + +### the_position_index_answers_what_the_walk_answered + +Compares `row_tuple`, `data_row_to_visual` and `row_folds_a_group` against the +walk over a grouped, collapsed and pruned axis, including two indices past the +end. Rival (`data_index_to_visual` reading `data_idx + 1`): fails at `visual 0`. + +### a_full_width_partial_probes_one_cell_instead_of_narrowing + +`model::cell::exact_path_tests`. A full-width partial must not reach set +narrowing, and an absent cell and an unseen item must match nothing. Rival +(`exact_positions` returning `None`): the narrowing count is 1, not 0. Rival +(`widest <= partial.len() + 1`): fails `a_wider_cell_turns_the_shortcut_off_until_it_is_removed`, +which stores a cell one coordinate wider and expects it matched. +`a_narrower_partial_still_narrows_and_matches_every_superset` is the control: the +shortcut is not a blanket exact lookup. `an_unsorted_full_width_partial_is_still_answered` +holds the canonical-order condition: a partial not in a stored key's order falls +back. + +### a_window_over_a_held_summary_equals_the_one_walk_grid_body + +`ui::viewport::tests`. The host keeps one `GridSummary` (widths, total formats, +totals) per layout and reads a window over it with `grid_window`; it must equal +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. diff --git a/crates/improvise-command/src/ui/app.rs b/crates/improvise-command/src/ui/app.rs index 45c74ad6..0419f863 100644 --- a/crates/improvise-command/src/ui/app.rs +++ b/crates/improvise-command/src/ui/app.rs @@ -467,8 +467,7 @@ pub struct App { pub term_width: u16, pub term_height: u16, /// **Derived cache**: a pure function of the slices, rebuilt by - /// `rebuild_layout()`. Owned by `App` because the rebuild is the host's job. - /// A writer that bypasses effects must call it: a keypress no longer does. + /// `rebuild_layout()`; a writer that bypasses effects must call it (felo.13). pub layout: GridLayout, /// How many times `rebuild_layout` has run: what a keypress that changes /// neither the model nor the view must leave alone (improvise-felo.13). @@ -941,9 +940,8 @@ impl App { self.geometry = std::cell::OnceCell::new(); } - /// Rebuild the layout only if something it is a function of has moved: the - /// model (`layout_stale`), the view it draws or its drill snapshot. A cursor - /// key moves none of them (improvise-felo.13, `context/code/command.md`). + /// Rebuild only if the model, the view drawn or the drill snapshot moved; a + /// cursor key moves none of them (layout-cost.md). fn refresh_layout(&mut self) { if self.layout_stale || self.layout_basis != self.current_layout_basis() { self.rebuild_layout(); @@ -953,7 +951,10 @@ impl App { fn current_layout_basis(&self) -> LayoutBasis { LayoutBasis { view: self.view_name().to_string(), - drill: self.drill_state.as_ref().map(|d| Rc::as_ptr(&d.records) as usize), + drill: self + .drill_state + .as_ref() + .map(|d| Rc::as_ptr(&d.records) as usize), } } @@ -1993,13 +1994,27 @@ mod tests { let key = CellKey::new(vec![("Row".into(), "A".into()), ("Col".into(), "X".into())]); let long = CellValue::Text("a value far wider than a column".into()); app.apply_effects(vec![Box::new(SetCell(key, long))]); - assert!(app.layout_builds > builds, "an edit must rebuild the layout"); + assert!( + app.layout_builds > builds, + "an edit must rebuild the layout" + ); app.dispatch_key(down()); assert!(app.width_passes.get() > widths, "an edit must remeasure"); let model = &app.model_state.workbook.model; - let fresh = crate::ui::viewport::compute_col_widths(model, &app.layout, &FormatPlan::of(model, app.view())); - assert_eq!(app.grid_geometry().summary.col_widths, fresh, "stale widths"); - assert_ne!(fresh, narrow, "the edit widened a column, or this proves nothing"); + let fresh = crate::ui::viewport::compute_col_widths( + model, + &app.layout, + &FormatPlan::of(model, app.view()), + ); + assert_eq!( + app.grid_geometry().summary.col_widths, + fresh, + "stale widths" + ); + assert_ne!( + fresh, narrow, + "the edit widened a column, or this proves nothing" + ); } /// improvise-felo.13: a `ViewOnly` effect that changes which view is drawn @@ -2010,7 +2025,9 @@ mod tests { app.model_state.workbook.create_view("Second"); app.rebuild_layout(); let builds = app.layout_builds; - app.apply_effects(vec![Box::new(crate::ui::effect::SwitchView("Second".into()))]); + app.apply_effects(vec![Box::new(crate::ui::effect::SwitchView( + "Second".into(), + ))]); assert_eq!(app.view_state.active_view.as_str(), "Second"); assert!(app.layout_builds > builds, "the layout still draws Default"); } diff --git a/crates/improvise-command/src/ui/project.rs b/crates/improvise-command/src/ui/project.rs index f9593b7e..3bdafcac 100644 --- a/crates/improvise-command/src/ui/project.rs +++ b/crates/improvise-command/src/ui/project.rs @@ -142,13 +142,17 @@ pub fn grid_in(app: &App, view_name: &str, viewport: Viewport) -> GridCache { let plan = FormatPlan::of(model, view); let rows = viewport.rows_in(layout.row_count()); let cols = viewport.cols_in(layout.col_count()); - // ONE walk: the widths, the totals and this window's values all come off the - // same evaluation — `context/code/command.md` (improvise-jmjz). - // The host's own layout keeps its whole-grid measurements until it is rebuilt, so - // a window costs its own cells. Another view's grid has no such cache. + // One walk gives widths, totals and window (improvise-jmjz); the host's layout + // keeps the first two until rebuilt, so its window costs its own cells. let hosted = elsewhere.is_none() && derived.is_none(); let body = match hosted { - true => grid_window(model, layout, &app.grid_geometry().summary, rows.clone(), cols.clone()), + true => grid_window( + model, + layout, + &app.grid_geometry().summary, + rows.clone(), + cols.clone(), + ), false => grid_body(model, layout, &plan, rows.clone(), cols.clone()), }; diff --git a/crates/improvise-command/src/ui/viewport.rs b/crates/improvise-command/src/ui/viewport.rs index 6ab2b84f..01d499b0 100644 --- a/crates/improvise-command/src/ui/viewport.rs +++ b/crates/improvise-command/src/ui/viewport.rs @@ -327,8 +327,16 @@ mod tests { let plan = FormatPlan::of(&wb.model, view); let summary = grid_summary(&wb.model, &layout, &plan); let (rows, cols) = (layout.row_count(), layout.col_count()); - assert!(rows >= 2 && cols >= 3, "a window strictly inside is possible"); - for (r, c) in [(0..rows, 0..cols), (1..2, 1..3), (rows - 1..rows, 0..1), (0..0, 0..0)] { + assert!( + rows >= 2 && cols >= 3, + "a window strictly inside is possible" + ); + for (r, c) in [ + (0..rows, 0..cols), + (1..2, 1..3), + (rows - 1..rows, 0..1), + (0..0, 0..0), + ] { let walked = grid_body(&wb.model, &layout, &plan, r.clone(), c.clone()); let held = grid_window(&wb.model, &layout, &summary, r.clone(), c.clone()); assert_eq!(held.window, walked.window, "window {r:?} x {c:?}"); @@ -337,5 +345,4 @@ mod tests { assert_eq!(held.col_formats, walked.col_formats); } } - } diff --git a/crates/improvise-core/src/model/cell.rs b/crates/improvise-core/src/model/cell.rs index e31c87a3..ab23eb70 100644 --- a/crates/improvise-core/src/model/cell.rs +++ b/crates/improvise-core/src/model/cell.rs @@ -212,8 +212,8 @@ pub struct DataStore { /// Reverse index for the client-facing surrogate (ADR 002), plus each /// record's write counter. Why it lives here: core.md §8. records: HashMap, - /// How many stored cells have each number of coordinates. Its largest key - /// says when a full-width partial can only match one exact cell (§8). + /// How many stored cells have each number of coordinates; its largest key + /// says when a partial can match only one exact cell. arities: std::collections::BTreeMap, } @@ -406,17 +406,16 @@ impl DataStore { .unwrap_or_default() } - /// The answer without narrowing, when `partial` names as many coordinates as - /// the widest stored cell has: only a cell equal to it can match (core.md §8). - /// `None` where that does not hold, and the narrowing path answers. + /// The answer without narrowing, when `partial` is as wide as the widest cell + /// stored: only an equal cell can match (layout-cost.md). `None` otherwise. fn exact_positions(&self, partial: &[(String, String)]) -> Option> { let widest = *self.arities.keys().next_back()?; // Strictly sorted by category, the order a stored key is in, so one probe is the lookup. let canonical = partial.windows(2).all(|w| w[0].0 < w[1].0); (!partial.is_empty() && widest <= partial.len() && canonical).then(|| { - let found = self.intern_partial(partial).and_then(|pairs| { - self.cells.get_index_of(&InternedKey(pairs)) - }); + let found = self + .intern_partial(partial) + .and_then(|pairs| self.cells.get_index_of(&InternedKey(pairs))); found.into_iter().collect() }) } @@ -1402,14 +1401,21 @@ mod exact_path_tests { use super::{CellKey, CellValue, DataStore, NARROWED}; fn key(a: &str, b: &str, c: &str) -> CellKey { - CellKey::new(vec![("A".into(), a.into()), ("B".into(), b.into()), ("C".into(), c.into())]) + CellKey::new(vec![ + ("A".into(), a.into()), + ("B".into(), b.into()), + ("C".into(), c.into()), + ]) } fn store() -> DataStore { let mut d = DataStore::new(); for i in 0..6 { for j in 0..6 { - d.set(key(&format!("a{i}"), &format!("b{j}"), "c"), CellValue::Number((i * 6 + j) as f64)); + d.set( + key(&format!("a{i}"), &format!("b{j}"), "c"), + CellValue::Number((i * 6 + j) as f64), + ); } } d @@ -1427,12 +1433,21 @@ mod exact_path_tests { let d = store(); let full = key("a2", "b3", "c").0; let mut got = Vec::new(); - assert_eq!(narrowings(|| got = d.matching_values(&full).into_iter().cloned().collect()), 0); + assert_eq!( + narrowings(|| got = d.matching_values(&full).into_iter().cloned().collect()), + 0 + ); assert_eq!(got, vec![CellValue::Number(15.0)]); let missing = key("a2", "b9", "c").0; - assert!(d.matching_values(&missing).is_empty(), "an absent cell matches nothing"); + assert!( + d.matching_values(&missing).is_empty(), + "an absent cell matches nothing" + ); let unknown = key("a2", "b3", "zzz").0; - assert!(d.matching_values(&unknown).is_empty(), "an unseen item matches nothing"); + assert!( + d.matching_values(&unknown).is_empty(), + "an unseen item matches nothing" + ); } /// The control: a narrower partial still sums its supersets, so the shortcut @@ -1440,7 +1455,10 @@ mod exact_path_tests { #[test] fn a_narrower_partial_still_narrows_and_matches_every_superset() { let d = store(); - let narrow = vec![("A".to_string(), "a2".to_string()), ("C".to_string(), "c".to_string())]; + let narrow = vec![ + ("A".to_string(), "a2".to_string()), + ("C".to_string(), "c".to_string()), + ]; let mut n = 0; assert_eq!(narrowings(|| n = d.matching_values(&narrow).len()), 1); assert_eq!(n, 6); @@ -1451,13 +1469,23 @@ mod exact_path_tests { fn a_wider_cell_turns_the_shortcut_off_until_it_is_removed() { let mut d = store(); let wide = CellKey::new(vec![ - ("A".into(), "a2".into()), ("B".into(), "b3".into()), ("C".into(), "c".into()), ("D".into(), "d".into()), + ("A".into(), "a2".into()), + ("B".into(), "b3".into()), + ("C".into(), "c".into()), + ("D".into(), "d".into()), ]); d.set(wide.clone(), CellValue::Number(100.0)); let full = key("a2", "b3", "c").0; - assert_eq!(d.matching_values(&full).len(), 2, "the exact cell and the wider cell under it"); + assert_eq!( + d.matching_values(&full).len(), + 2, + "the exact cell and the wider cell under it" + ); d.remove(&wide); - assert_eq!(narrowings(|| assert_eq!(d.matching_values(&full).len(), 1)), 0); + assert_eq!( + narrowings(|| assert_eq!(d.matching_values(&full).len(), 1)), + 0 + ); } /// An unsorted partial is not a stored key's order, so it must not be probed. diff --git a/crates/improvise-core/src/view/layout.rs b/crates/improvise-core/src/view/layout.rs index 68e577a0..269745b5 100644 --- a/crates/improvise-core/src/view/layout.rs +++ b/crates/improvise-core/src/view/layout.rs @@ -252,9 +252,8 @@ thread_local! { static DATA_ITEM_STEPS: std::cell::Cell = const { std::cell::Cell::new(0) }; } -/// Where each data item sits in `items`: entry `positions(items)[n]` is data item -/// `n`. Built once per axis so a lookup by data index is O(1), not a walk past -/// every header and item before it (improvise-felo.13, `core.md` §20). +/// Entry `positions(items)[n]` is data item `n`: a lookup by data index is O(1), +/// not a walk past every header before it (layout-cost.md). fn positions(items: &[AxisEntry]) -> Vec { items .iter() @@ -3083,12 +3082,26 @@ mod lookup_cost_tests { wb.active_view_mut().toggle_group_collapse("Row", &["Q2"]); wb.active_view_mut().prune_empty = true; for i in 0..5 { - let key = CellKey::new(vec![("Row".into(), format!("r{i}")), ("Col".into(), "c0".into())]); + let key = CellKey::new(vec![ + ("Row".into(), format!("r{i}")), + ("Col".into(), "c0".into()), + ]); wb.model.set_cell(key, CellValue::Number(1.0)); } let layout = GridLayout::with_frozen_records(&wb.model, wb.active_view(), None); - assert!(layout.row_items.iter().any(|e| !matches!(e, AxisEntry::DataItem { .. }))); - assert!(layout.row_items.iter().any(|e| matches!(e, AxisEntry::DataItem { collapsed: true, .. }))); + assert!( + layout + .row_items + .iter() + .any(|e| !matches!(e, AxisEntry::DataItem { .. })) + ); + assert!(layout.row_items.iter().any(|e| matches!( + e, + AxisEntry::DataItem { + collapsed: true, + .. + } + ))); let walked: Vec<&Vec> = data_items(&layout.row_items).collect(); assert_eq!(layout.row_count(), walked.len()); let visual: Vec = layout @@ -3099,19 +3112,31 @@ mod lookup_cost_tests { .map(|(at, _)| at) .collect(); for i in 0..walked.len() + 2 { - assert_eq!(layout.row_tuple(i), walked.get(i).map(|t| t.as_slice()), "row {i}"); - assert_eq!(layout.data_row_to_visual(i), visual.get(i).copied(), "visual {i}"); + assert_eq!( + layout.row_tuple(i), + walked.get(i).map(|t| t.as_slice()), + "row {i}" + ); + assert_eq!( + layout.data_row_to_visual(i), + visual.get(i).copied(), + "visual {i}" + ); let folded = matches!( data_items_entry(&layout.row_items, i), - Some(AxisEntry::DataItem { collapsed: true, .. }) + Some(AxisEntry::DataItem { + collapsed: true, + .. + }) ); assert_eq!(layout.row_folds_a_group(i), folded, "fold {i}"); } } fn data_items_entry(items: &[super::AxisEntry], n: usize) -> Option<&super::AxisEntry> { - items.iter().filter(|e| matches!(e, super::AxisEntry::DataItem { .. })).nth(n) + items + .iter() + .filter(|e| matches!(e, super::AxisEntry::DataItem { .. })) + .nth(n) } - } -