Point the geometry test at the production hash, and test both abort arms

improvise-1xf4: `layout_geometry_effects_actually_change_view_config`
hand-listed 6 of the 13 LayoutGeometry effects and measured them against
`view_config_fingerprint`, a THIRD fingerprint existing only in test code
beside the production `document_config` and `view_shape`. Since PR #70
decides `model_state.dirty` for LayoutGeometry solely from
`document_config`, an effect writing something the test-only hash sees
and `document_config` does not would pass and never mark the document
dirty — i.e. never be saved.

It is now `layout_geometry_effects_actually_change_document_config`,
measured against `document_config` itself and listing all 13, counted off
the `impl Effect` blocks rather than trusted. Two fixture facts the
rewrite exposed and the old six never could: `ShowItem` on a visible item
and `ToggleGroup` on a nonexistent group are no-ops, so `grouped_app`
supplies a hidden item, a real group and a second view.

`view_config_fingerprint` SURVIVES, deliberately, and is not a third
derivation any more: it is used only by the NEGATIVE arm
(`view_only_effects_leave_shared_state_alone`), where a hash stricter
than `document_config` is what that test wants. The doc comment says so.

improvise-2avl: PR #70 moved the `layout_version` bump out of the effect
loop. Gotcha 40 records the in-loop position as load-bearing, and the
argument that end-of-batch is safe for `layout_version` specifically was
written into context/code/command.md and never tested. Both arms now are:
a batch that reshaped before aborting must bump, one that aborted before
reshaping must not. Proved independently — bumping unconditionally fails
only the second, skipping `observe_reshape` on abort fails only the first.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH

crates/improvise-command/src/ui/app.rs

@@ -82,7 +82,7 @@ fn view_shapes(workbook: &Workbook) -> std::collections::BTreeMap<String, u64> {
 
 /// Everything a `LayoutGeometry` effect can write that the file would carry. Not the
 /// cell data — `record_fingerprints` answers for that — so it is cheap per batch.
-fn document_config(workbook: &Workbook) -> u64 {
+pub(crate) fn document_config(workbook: &Workbook) -> u64 {
     use std::hash::{Hash, Hasher};
     let mut h = std::collections::hash_map::DefaultHasher::new();
     for (name, cat) in &workbook.model.categories {

crates/improvise-command/src/ui/effect.rs

@@ -1326,7 +1326,9 @@ mod tests {
         serde_json::to_string(&app.model_state.workbook.model).expect("model serializes")
     }
 
-    /// Serialized view configuration (axes, pages, hidden items, format).
+    /// Serialized view configuration. Kept for the NEGATIVE arm only, where a
+    /// stricter hash than `document_config` is what the test wants — the
+    /// positive arm is `document_config`'s (improvise-1xf4).
     fn view_config_fingerprint(app: &App) -> String {
         serde_json::to_string(&app.model_state.workbook.views).expect("views serialize")
     }
@@ -1482,38 +1484,80 @@ mod tests {
         }
     }
 
-    /// Effects declaring `LayoutGeometry` must change view configuration —
-    /// the thing that makes every client re-lay out its grid.
+    /// Effects declaring `LayoutGeometry` must move `document_config` — the
+    /// production hash `model_state.dirty` is decided by (improvise-1xf4).
     #[test]
-    fn layout_geometry_effects_actually_change_view_config() {
+    fn layout_geometry_effects_actually_change_document_config() {
         let cases: Vec<Box<dyn Effect>> = vec![
             Box::new(AddCategory("Region".into())),
             Box::new(RemoveCategory("Month".into())),
+            Box::new(CreateView("Second".into())),
+            Box::new(DeleteView("Default".into())),
             Box::new(SetAxis {
                 category: "Type".into(),
                 axis: Axis::Page,
             }),
+            Box::new(SetPageSelection {
+                category: "Type".into(),
+                item: "Clothing".into(),
+            }),
+            Box::new(ToggleGroup {
+                category: "Type".into(),
+                group: "Wearables".into(),
+            }),
+            Box::new(HideItem {
+                category: "Type".into(),
+                item: "Food".into(),
+            }),
+            Box::new(ShowItem {
+                category: "Type".into(),
+                item: "Clothing".into(),
+            }),
             Box::new(TransposeAxes),
+            Box::new(CycleAxis("Type".into())),
             Box::new(SetNumberFormat(",.3f".into())),
             Box::new(TogglePruneEmpty),
         ];
+        // All thirteen, counted off the `impl Effect` blocks rather than trusted:
+        // a fourteenth declaring LayoutGeometry must come here and say so.
+        assert_eq!(cases.len(), 13, "the LayoutGeometry effects are thirteen");
         for effect in cases {
             assert_eq!(
                 effect.kind(),
                 EffectKind::Model(ModelChange::LayoutGeometry),
                 "{effect:?} is in the LayoutGeometry case list"
             );
-            let mut app = test_app();
-            let before = view_config_fingerprint(&app);
+            let mut app = grouped_app();
+            let before = crate::ui::app::document_config(&app.model_state.workbook);
             effect.apply(&mut app);
             assert_ne!(
-                view_config_fingerprint(&app),
+                crate::ui::app::document_config(&app.model_state.workbook),
                 before,
-                "{effect:?} declares LayoutGeometry but changed no view config"
+                "{effect:?} declares LayoutGeometry but did not move document_config, \
+                 so a batch carrying only it would never mark the document dirty"
             );
         }
     }
 
+    /// `test_app` plus the three things the thirteen need in order to have
+    /// something to change: a second view to delete, a group to collapse, and
+    /// one already-hidden item, so `ShowItem` is not a no-op.
+    fn grouped_app() -> App {
+        let mut app = test_app();
+        {
+            let wb = &mut app.model_state.workbook;
+            wb.model
+                .category_mut("Type")
+                .unwrap()
+                .add_item_in_group("Hats", "Wearables");
+            wb.create_view("Spare");
+            wb.switch_view("Default").unwrap();
+            wb.active_view_mut().hide_item("Type", "Clothing");
+        }
+        app.rebuild_layout();
+        app
+    }
+
     // ── Model mutation effects ──────────────────────────────────────────
 
     #[test]
@@ -1729,6 +1773,53 @@ mod tests {
         assert!(app.view_state.view_back_stack.is_empty());
     }
 
+    /// improvise-2avl. PR #70 moved the `layout_version` bump out of the effect
+    /// loop; gotcha 40 records the in-loop position as load-bearing, and the
+    /// argument that it is safe HERE was written down and not tested. Both arms.
+    #[test]
+    fn an_aborted_batch_bumps_layout_version_for_what_it_did_run() {
+        let mut app = test_app();
+        let before = app.layout_version();
+        // The reshape lands, THEN the abort. `observe_reshape` runs after the
+        // break, so it must still see the axis move.
+        app.apply_effects(vec![
+            Box::new(SetAxis {
+                category: "Type".into(),
+                axis: Axis::Page,
+            }),
+            Box::new(AbortChain),
+            Box::new(TogglePruneEmpty),
+        ]);
+        assert!(app.last_batch_aborted(), "the fixture must actually abort");
+        assert_eq!(
+            app.layout_version(),
+            before + 1,
+            "a batch that reshaped before aborting still moved the grid, and a \
+             client composing against the old version must be told"
+        );
+    }
+
+    #[test]
+    fn an_aborted_batch_bumps_nothing_for_effects_it_never_ran() {
+        let mut app = test_app();
+        let before = app.layout_version();
+        // The abort comes FIRST, so the reshaping effect behind it never runs.
+        app.apply_effects(vec![
+            Box::new(AbortChain),
+            Box::new(SetAxis {
+                category: "Type".into(),
+                axis: Axis::Page,
+            }),
+        ]);
+        assert!(app.last_batch_aborted(), "the fixture must actually abort");
+        assert_eq!(
+            app.layout_version(),
+            before,
+            "nothing reshaped, so no client's basis went stale — an over-bump \
+             here is a rejection somebody did not earn"
+        );
+    }
+
     #[test]
     fn view_back_and_forward() {
         let mut app = test_app();