fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Give RecordRewrite's three untested branches a test that can fail (improvise-phg1)

Browse Source 
Three RecordRewrite branches could be broken with the whole workspace still
green:
- the lone measure-less cell of an `o`-seeded row;
- a measure a sparse drill row has no cell for, which must be written at the
  renamed record;
- every row reading the model before any row clears it.

Each test is red against the rival the review injected and fails alone. No
production code changes.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
Edward Langley
2026-09-22 23:00:08 -07:00
parent 5c50bf8780
commit e829b66487
2 changed files with 136 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files
context/test-evidence/command.md
+34
View File
@@ -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.
crates/improvise-command/src/ui/effect.rs
+102 -20
View File
@@ -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<String> =
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(