diff --git a/context/test-evidence/command.md b/context/test-evidence/command.md index c2a58df7..5744f864 100644 --- a/context/test-evidence/command.md +++ b/context/test-evidence/command.md @@ -5394,3 +5394,37 @@ Rivals, injected, full workspace run: *else* fails: `renders_each_tile_with_its_axis_label` (TUI) and `the_tile_bar_route_into_records_mode_does_not_strand_the_user`. The second one is improvise-bd9o's own test of the route this decision keeps. + +### renaming_a_drill_row_held_by_a_lone_measureless_cell_moves_that_cell + +improvise-phg1(a). A row seeded by `o` (`AddRecordRow`) carries its coordinates +on one cell with no `_Measure`, and `RecordRewrite::of` moves that cell in its +`(false, true)` arm. Replacing the arm with `Vec::new()` left the whole workspace +green, and a renamed `o` row then left `CellKey([])` behind with no coordinates. +The gap predates improvise-uo2p, because `rename_record_coordinate` had the same +arm and nothing tested it. The fixture is that literal case: an empty-coordinate +cell on drill row 1, renamed. Under the rival it fails with `-/-/- = 0` still in +the store, and nothing else in the workspace fails. + +### a_measure_a_sparse_drill_row_lacks_is_written_at_the_renamed_record + +improvise-phg1(b). An edit to a measure the row has no cell for goes through +`of`'s second loop, which must write at the **renamed** key. Every uo2p fixture +row carried every measure, so that loop only ever ran without a rename, and +writing `row.cell_key(measure)` unrenamed brought uo2p's ghost record back +unseen. Here drill row 1 has `Revenue` but not `Cost`, and stages `Cost` and a +rename. Under the rival it fails with a `Clothing/Jan/Cost = 999` ghost, and +nothing else in the workspace fails. + +### every_drill_row_reads_the_model_before_any_row_clears_it + +improvise-phg1(c). `RecordRewrite::merged` puts every clear after every read. +The rival lets each row's clears land before the next row reads the model, and +the uo2p swap test does not see it: a row's own clears touch only its own old +keys, which no other row reads. What separates the two is a clear aimed at +**another row's** key. A blank edit on a measure the row lacks clears that +measure at the row's *renamed* coordinates. So row 0 is renamed onto row 1's +coordinates and blanks `Profit`, which only row 1 has, and row 1 is renamed +away. Row 1's `Profit` must still arrive at `Food/Jan`. Under the rival it is +cleared before row 1 reads it, `Food/Jan/Profit = 5` is lost, and nothing else +in the workspace fails. This probe is the reviewer's, from the ticket. diff --git a/crates/improvise-command/src/ui/effect.rs b/crates/improvise-command/src/ui/effect.rs index 6eefb8f9..9976f280 100644 --- a/crates/improvise-command/src/ui/effect.rs +++ b/crates/improvise-command/src/ui/effect.rs @@ -3143,26 +3143,9 @@ mod tests { /// Two records, each carrying Revenue and Cost: row 0 is Food/Jan (1, 2), /// row 1 is Clothing/Jan (42, 17). Every edit below is staged on row 1 at least. fn two_record_drill() -> App { - let mut app = test_app(); - let cells = [ - (drill_cell("Food", "Jan", "Revenue"), CellValue::Number(1.0)), - (drill_cell("Food", "Jan", "Cost"), CellValue::Number(2.0)), - ( - drill_cell("Clothing", "Jan", "Revenue"), - CellValue::Number(42.0), - ), - ( - drill_cell("Clothing", "Jan", "Cost"), - CellValue::Number(17.0), - ), - ]; - for (key, value) in cells.clone() { - app.model_state.workbook.model.set_cell(key, value); - } - let records = RecordRow::group(cells); - assert_eq!(records.len(), 2, "one row per coordinate tuple"); + let app = drill_of(&two_record_drill_cells()); + let records = &app.drill_state.as_ref().unwrap().records; assert_eq!(records[1].coords.get("Type"), Some("Clothing")); - StartDrill(records).apply(&mut app); app } @@ -3206,10 +3189,49 @@ mod tests { /// Stage `edits` and leave the drill, in 32 fresh Apps: each `HashMap` gets its /// own `RandomState`, and one map iterates the same way twice. fn assert_drill_commits_to(edits: &[(usize, RecordCol, &str)], expected: &[&str]) { + let cells = two_record_drill_cells(); + assert_drill_over(&cells, edits, expected); + } + + fn two_record_drill_cells() -> Vec<(CellKey, CellValue)> { + vec![ + (drill_cell("Food", "Jan", "Revenue"), CellValue::Number(1.0)), + (drill_cell("Food", "Jan", "Cost"), CellValue::Number(2.0)), + ( + drill_cell("Clothing", "Jan", "Revenue"), + CellValue::Number(42.0), + ), + ( + drill_cell("Clothing", "Jan", "Cost"), + CellValue::Number(17.0), + ), + ] + } + + /// A drill over exactly `cells`, which must group into two records. + fn drill_of(cells: &[(CellKey, CellValue)]) -> App { + let mut app = test_app(); + for (key, value) in cells { + app.model_state + .workbook + .model + .set_cell(key.clone(), value.clone()); + } + let records = RecordRow::group(cells.iter().cloned()); + assert_eq!(records.len(), 2, "one row per coordinate tuple"); + StartDrill(records).apply(&mut app); + app + } + + fn assert_drill_over( + cells: &[(CellKey, CellValue)], + edits: &[(usize, RecordCol, &str)], + expected: &[&str], + ) { let expected: std::collections::BTreeSet = expected.iter().map(|s| s.to_string()).collect(); for attempt in 0..32 { - let mut app = two_record_drill(); + let mut app = drill_of(cells); for (row, col, value) in edits { stage(&mut app, *row, col.clone(), value); } @@ -3218,6 +3240,66 @@ mod tests { } } + #[test] + fn renaming_a_drill_row_held_by_a_lone_measureless_cell_moves_that_cell() { + assert_drill_over( + &[ + (drill_cell("Food", "Jan", "Revenue"), CellValue::Number(1.0)), + (CellKey::new(vec![]), CellValue::Number(0.0)), + ], + &[(1, category("Type"), "Drink")], + &["Food/Jan/Revenue = 1", "Drink/-/- = 0"], + ); + } + + #[test] + fn a_measure_a_sparse_drill_row_lacks_is_written_at_the_renamed_record() { + assert_drill_over( + &[ + (drill_cell("Food", "Jan", "Revenue"), CellValue::Number(1.0)), + (drill_cell("Food", "Jan", "Cost"), CellValue::Number(2.0)), + ( + drill_cell("Clothing", "Jan", "Revenue"), + CellValue::Number(42.0), + ), + ], + &[(1, measure("Cost"), "999"), (1, category("Type"), "Drink")], + &[ + "Food/Jan/Revenue = 1", + "Food/Jan/Cost = 2", + "Drink/Jan/Revenue = 42", + "Drink/Jan/Cost = 999", + ], + ); + } + + #[test] + fn every_drill_row_reads_the_model_before_any_row_clears_it() { + assert_drill_over( + &[ + (drill_cell("Food", "Jan", "Revenue"), CellValue::Number(1.0)), + ( + drill_cell("Clothing", "Jan", "Revenue"), + CellValue::Number(42.0), + ), + ( + drill_cell("Clothing", "Jan", "Profit"), + CellValue::Number(5.0), + ), + ], + &[ + (0, category("Type"), "Clothing"), + (0, measure("Profit"), ""), + (1, category("Type"), "Food"), + ], + &[ + "Clothing/Jan/Revenue = 1", + "Food/Jan/Revenue = 42", + "Food/Jan/Profit = 5", + ], + ); + } + #[test] fn a_drill_row_renamed_twice_and_edited_lands_as_one_record() { assert_drill_commits_to(