Guard App::restore's rebuild-after-view_state order
improvise-3til: rebuilding before view_state is restored passed every test. A rolled-back frame that deleted the host's view now asserts the host is back on that view with that view's layout shape. 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:
@@ -6239,3 +6239,27 @@ behavioural one is the reason the rebuild exists.
|
||||
Placed in `ui/host.rs` rather than in `ui/app.rs`'s `snapshot_restore` module
|
||||
(which PR #254 adds), both to avoid that conflict and because the consequence is
|
||||
the host's.
|
||||
|
||||
### a_rolled_back_delete_of_the_hosts_view_rebuilds_that_views_layout
|
||||
|
||||
improvise-3til, found in the adversarial review of PR #261. `App::restore`
|
||||
rebuilds the layout **last**, after `view_state` is restored. The order matters
|
||||
because `rebuild_layout` reads `view_name()`, which comes from
|
||||
`view_state.active_view`. Moving the rebuild up to just after the workbook
|
||||
assignment passed every test in `improvise-command`, including
|
||||
`a_frame_after_a_rolled_back_reshape_resolves_against_the_restored_grid`,
|
||||
because that frame never moves the host's view.
|
||||
|
||||
This test moves it. The host sits on a transposed view, `Other` (1 row by 2
|
||||
columns). A client's frame `[DeleteView(Other), RemoveItem(South),
|
||||
SetCell(South)]` deletes the host's view, which moves the host to the default
|
||||
view, and is then refused `ChainBroken` and rolled back. The test asserts that
|
||||
the host is back on `Other` **and** that its layout is Other's shape.
|
||||
|
||||
- **Rival** (the bead's: rebuild before `view_state` is restored), run across
|
||||
the whole workspace: fails **only** this test. It fails at the layout shape,
|
||||
(2, 1) against (1, 2), because the rival builds the default view's grid and
|
||||
then puts the host back on `Other`.
|
||||
- The bead suggests following up with a write that resolves only through
|
||||
Other's layout. In this fixture both views show the same two cells, so no
|
||||
write can tell the two layouts apart. The shape is the discriminating fact.
|
||||
|
||||
@@ -1033,6 +1033,63 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// A rolled-back frame that moved the HOST's view rebuilds the layout of the
|
||||
/// view it is put back on (improvise-3til). Evidence: test-evidence/command.md.
|
||||
#[test]
|
||||
fn a_rolled_back_delete_of_the_hosts_view_rebuilds_that_views_layout() {
|
||||
use improvise_protocol::ModelCommand;
|
||||
|
||||
let mut host = host();
|
||||
let other = host.app.model_state.workbook.create_view("Other");
|
||||
other.set_axis("Region", Axis::Column);
|
||||
other.set_axis("_Measure", Axis::Row);
|
||||
assert!(
|
||||
host.app.switch_to_view("Other"),
|
||||
"setup: the host is on Other"
|
||||
);
|
||||
host.app.rebuild_layout();
|
||||
let (asker, _) = host.connect(10, 10);
|
||||
let shape = |host: &Host| (host.app().layout.row_count(), host.app().layout.col_count());
|
||||
assert_eq!(shape(&host), (1, 2), "setup: Other is transposed");
|
||||
|
||||
let basis = frame_basis(&host, &["South"]);
|
||||
let refused = host.handle(
|
||||
asker,
|
||||
ClientFrame::Commands(vec![
|
||||
Command::Model {
|
||||
basis: basis.clone(),
|
||||
cmd: ModelCommand::DeleteView("Other".into()),
|
||||
},
|
||||
Command::Model {
|
||||
basis: basis.clone(),
|
||||
cmd: ModelCommand::RemoveItem {
|
||||
category: "Region".into(),
|
||||
item: "South".into(),
|
||||
},
|
||||
},
|
||||
Command::Model {
|
||||
basis,
|
||||
cmd: ModelCommand::SetCell {
|
||||
cell: addressed("South"),
|
||||
value: CellValue::Number(5.0),
|
||||
},
|
||||
},
|
||||
]),
|
||||
);
|
||||
assert!(
|
||||
refused
|
||||
.iter()
|
||||
.any(|a| matches!(a.msg, ServerFrame::Rejected(Rejection::ChainBroken(_)))),
|
||||
"setup: the frame must be refused and rolled back: {refused:?}"
|
||||
);
|
||||
assert_eq!(host.app().view_name(), "Other", "the host is back on Other");
|
||||
assert_eq!(
|
||||
shape(&host),
|
||||
(1, 2),
|
||||
"and its layout must be Other's, not the default's"
|
||||
);
|
||||
}
|
||||
|
||||
/// The rollback reaches the MIRRORS, not only the document — the cost the
|
||||
/// ticket called its least obvious. See `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user