chore: address Copilot review on #6 (improvise-toc)

Two findings from the review of #6, which merged before they could be
applied there.

The gotcha I added to repo-map.md was numbered 31 but inserted above
gotcha 30, so the list read 29, 31, 30. Moved it below 30.

The wizard test helper wrapped MemoryFs in an Arc behind a local Storage
type that only forwarded to it. That pattern is load-bearing in
effect.rs, where the test inspects the backend after the App has used it,
but this test never looks at the backend again — and MemoryFs writes
through &self, so it can be seeded and handed straight to
App::with_storage. 18 lines and one indirection gone.

Workspace green, clippy clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DyHK7L8S1XDX3EihTnSKaX

context/repo-map.md

@@ -177,10 +177,10 @@ Region=West, Measure=Revenue = |pending|   # pipe-quoted text
 
 29. **"Which view is active" is stored twice, and the copies must move together.** `Workbook::active_view` is the document's; `ViewState::active_view` is the client's, and it is what keys `ViewState::cursors`. The second copy exists because `reduce_view` runs on a client that holds no `Workbook` (improvise-cqi). Only two writers are allowed to touch either — `App::switch_to_view` and `App::install_workbook` — and `the_two_active_view_names_never_diverge` guards it. Calling `workbook.switch_view()` directly desyncs them and the cursor map is then read under the wrong key; that is exactly how the 8uz characterization test broke during the extraction. Whether the two are *allowed* to diverge — two clients on different views — is improvise-g0f, deliberately unanswered here.
 
-31. **`:` commands settle their mode by observation, not prediction.** `ExecuteCommand` ends every batch with `ReturnToNormalUnlessMoved(ctx.mode)`, which drops to `Normal` only if the mode is still what the batch started in. There is no `Effect::changes_mode()` any more: it was a per-type constant, and `ViewBack`/`ViewForward`/`StartImportWizard` swap the mode only on some runs, so no up-front answer was right — `:import` opened the wizard and was snapped straight back out (improvise-toc). A new effect that changes the mode needs no annotation; just don't return the batch's starting mode from a success path.
-
 30. **Two tests gate `ViewState` membership.** `ViewState` is the *portable* session slice — the one thing serialized into the Snapshot a newly-connected client gets (improvise-q08). A field may live there only if it is **portable** (plain data from the allowlisted crates, no `Rc`/`Arc`) and **snapshot-safe** (a fresh client inheriting the value is correct). `wizard` fails both — `ImportWizard` is an `improvise-io` type, and a half-open dialog is not something to inherit — and `drill_state` fails both — it holds an `Rc`, and `pending_edits` are uncommitted writes belonging to whoever staged them. Both live on `App` under the **session-local** residue tag (improvise-h0b). The `ViewState: Send` static assertion in `app.rs`'s tests is the compile-time proxy for portability: `Rc` is not `Send`, so a re-added shared pointer fails the build (the `Deserialize` derive from gotcha 25 now rejects it too, serde's `rc` feature being off). `Send` does *not* catch the origin-crate or snapshot-safety halves — check those by hand.
 
+31. **`:` commands settle their mode by observation, not prediction.** `ExecuteCommand` ends every batch with `ReturnToNormalUnlessMoved(ctx.mode)`, which drops to `Normal` only if the mode is still what the batch started in. There is no `Effect::changes_mode()` any more: it was a per-type constant, and `ViewBack`/`ViewForward`/`StartImportWizard` swap the mode only on some runs, so no up-front answer was right — `:import` opened the wizard and was snapped straight back out (improvise-toc). A new effect that changes the mode needs no annotation; just don't return the batch's starting mode from a success path.
+
 ---
 
 ## File Inventory

crates/improvise-command/src/command/cmd/text_buffer.rs

@@ -169,29 +169,13 @@ mod tests {
         use crate::storage::{MemoryFs, Storage};
         use crate::ui::app::App;
         use std::path::Path;
-        use std::sync::Arc;
 
-        #[derive(Debug)]
-        struct SharedMemoryFs(Arc<MemoryFs>);
-        impl Storage for SharedMemoryFs {
-            fn read(&self, path: &Path) -> anyhow::Result<Vec<u8>> {
-                self.0.read(path)
-            }
-            fn write(&self, path: &Path, bytes: &[u8]) -> anyhow::Result<()> {
-                self.0.write(path, bytes)
-            }
-            fn list(&self, dir: &Path) -> anyhow::Result<Vec<std::path::PathBuf>> {
-                self.0.list(dir)
-            }
-            fn delete(&self, path: &Path) -> anyhow::Result<()> {
-                self.0.delete(path)
-            }
-        }
-
-        let fs = Arc::new(MemoryFs::new());
+        // MemoryFs writes through &self, so it can be seeded and then handed
+        // straight to the App — the test never reads the backend again.
+        let fs = MemoryFs::new();
         fs.write(Path::new("d.json"), br#"[{"region":"East","rev":1}]"#)
             .unwrap();
-        App::with_storage(two_cat_model(), None, Box::new(SharedMemoryFs(fs)))
+        App::with_storage(two_cat_model(), None, Box::new(fs))
     }
 
     /// Drive a `:` command line through the full command → effect → App path