Merge pull request #41 from fiddlerwoaroof/comments/command

docs(command): move improvise-command's reasoning out of comments into context/
2026-09-20 10:43:18 -07:00
parent 04e4e3de78 288cbca8cf
commit 5f871f7035
26 changed files with 5026 additions and 3482 deletions
@@ -114,8 +114,7 @@ mod tests {
 }
 // ── Cell operations ──────────────────────────────────────────────────────────
-// All cell commands take an explicit CellKey. The interactive spec fills it
+// All cell commands take an explicit CellKey; see `context/code/command.md`.
 // from ctx.cell_key(); the parser fills it from Cat/Item coordinate args.
 /// Clear a cell.
 #[derive(Debug)]
@@ -64,13 +64,8 @@ mod tests {
        );
    }
-    /// improvise-6t3: the commit pushed `set_status("Formula added")` *after*
+    /// improvise-6t3: `set_status("Formula added")` after `AddFormula` overwrote the
-    /// `AddFormula`, so the parse error that `AddFormula::apply` writes to the
+    /// parse error, so an unparseable formula was dropped and reported a success.
    /// status bar was immediately overwritten. An unparseable formula was
    /// dropped while the UI reported success.
    ///
    /// The add effect is the only party that knows whether it worked, so it has
    /// to get the last word on the status line.
    #[test]
    fn invalid_formula_reports_the_parse_error_not_success() {
        let m = two_cat_model();
@@ -158,9 +153,8 @@ mod tests {
        );
    }
-    /// `CommitAndAdvance` must thread its `edit_mode` through to the
+    /// `CommitAndAdvance` threads its `edit_mode` to the trailing
-    /// trailing `EnterEditAtCursor` effect so the post-commit re-edit lands
+    /// `EnterEditAtCursor`; the command never reads `ctx.mode`.
    /// in the mode the keymap requested. The command never reads ctx.mode.
    #[test]
    fn commit_and_advance_threads_edit_mode_to_enter_edit_at_cursor() {
        let m = two_cat_model();
@@ -203,14 +197,8 @@ mod tests {
        assert!(dbg.contains("Normal"), "Expected Normal mode, got: {dbg}");
    }
-    /// improvise-lae: the save prompt has to commit to `SaveAs` and not to
+    /// improvise-lae: the save prompt commits to `SaveAs`, not `ExportCsv` (wrong
-    /// `ExportCsv`, which is what makes it a separate mode from
+    /// format) and not `Save` (there is no path yet).
    /// `ExportPrompt` rather than a second binding onto it — `ExportCsv`
    /// would write CSV under whatever `.improv` name the user typed.
    ///
    /// `SaveAs` and not `Save`, because `Save` writes the path the document
    /// already has, and the whole reason this prompt exists is that there
    /// isn't one.
    #[test]
    fn commit_save_writes_the_model_to_the_typed_path() {
        let m = two_cat_model();
@@ -232,9 +220,8 @@ mod tests {
        assert!(dbg.contains("Normal"), "Expected Normal mode, got: {dbg}");
    }
-    /// An empty prompt must not reach `SaveAs`: `SaveAs(PathBuf::from(""))`
+    /// An empty prompt must not reach `SaveAs`: the user would read an I/O error
-    /// asks the backend to write to `""`, and the user then reads whatever
+    /// about `""` instead of "you typed nothing".
    /// I/O error that produces instead of "you typed nothing".
    #[test]
    fn commit_save_on_an_empty_path_writes_nothing() {
        let m = two_cat_model();
@@ -255,9 +242,8 @@ mod tests {
 // ── Commit commands (mode-specific buffer consumers) ────────────────────────
-/// Commit a cell value: for synthetic records keys, stage in drill pending edits
+/// Commit a cell value: a synthetic records key stages into the drill or applies
-/// in drill mode, or apply directly in plain records mode; for real keys, write
+/// directly in plain records mode; a real key writes to the model (gotcha 19).
 /// to the model.
 fn commit_regular_cell_value(key: &CellKey, value: &str, effects: &mut Vec<Box<dyn Effect>>) {
    if value.is_empty() {
        effects.push(Box::new(effect::ClearCell(key.clone())));
@@ -280,9 +266,8 @@ fn stage_drill_edit(record_idx: usize, col: RecordCol, value: &str) -> Box<dyn E
    })
 }
-/// Rename one coordinate of a records row. In wide format a row is a group
+/// Rename one coordinate of a records row. In wide format a row is a group of cells,
-/// of cells — one per measure — so the rename has to move every one of them,
+/// so the rename moves **every** one of them (improvise-c5v, gotcha 1).
 /// not just the cell under the cursor (improvise-c5v).
 fn rename_record_coordinate(
    model: &crate::model::Model,
    row: &RecordRow,
@@ -299,9 +284,8 @@ fn rename_record_coordinate(
        item: value.to_string(),
    }));
-    // Every cell in the row moves. A row seeded by `add-record-row` has no
+    // Every cell in the row moves. A row seeded by `add-record-row` carries its
-    // measure cells yet — its coordinates live on a lone measure-less cell,
+    // coordinates on a lone measure-less cell, which must move too.
    // which must move too or the row loses its identity.
    let cells: Vec<(CellKey, CellValue)> = if row.values.is_empty() {
        model
            .get_cell(&row.coords)
@@ -345,9 +329,8 @@ fn commit_plain_records_edit(
    }
 }
-/// Typing a name into the trailing column creates that measure, which gives
+/// Typing a name into the trailing column creates that measure. A schema change, so
-/// it a real column to type values into. Adding a measure is a schema
+/// it applies immediately even inside a drill.
 /// change, so it applies immediately even inside a drill.
 fn add_measure(value: &str, effects: &mut Vec<Box<dyn Effect>>) {
    let name = value.trim();
    if name.is_empty() {
@@ -359,20 +342,8 @@ fn add_measure(value: &str, effects: &mut Vec<Box<dyn Effect>>) {
    }));
 }
-/// Typing a name into the `+cat` column creates that category, which gives
+/// Typing a name into `+cat` creates the category **and its column**, and writes no
-/// the row a coordinate column to type into (improvise-2zq).
+/// coordinate on the current row (improvise-2zq) — see `context/code/command.md`.
 ///
 /// It creates the column and **does not set the coordinate on the current
 /// row**, which is the deliberate answer to the ticket's third question.
 /// Two reasons. It is symmetric with the measure `+`, which creates an empty
 /// column and writes no value. And setting it would move *every* measure
 /// cell in the row onto the new coordinate (gotcha 1) — a data migration
 /// across N cells attached to the same Enter that made a schema change, in
 /// an app with no undo. The follow-up edit is one `l` away in the column
 /// that just appeared.
 ///
 /// Like `add_measure` this is a schema change, so it applies immediately
 /// even inside a drill rather than being staged.
 fn add_category(value: &str, effects: &mut Vec<Box<dyn Effect>>) {
    let name = value.trim();
    if name.is_empty() {
@@ -405,13 +376,8 @@ fn commit_cell_value(
    commit_regular_cell_value(key, value, effects);
 }
-/// Whether committing `key` inserts a column, invalidating every column
+/// Whether committing `key` inserts a column, invalidating every index to its right.
-/// index to its right.
+/// Asked of the *layout*, so there is one answer to "what is this column" (gotcha 1).
 ///
 /// Asked of the *layout* — `record_col` resolves the synthetic `_Dim`
 /// position into what the column actually is — rather than re-derived from
 /// the value or the cursor, so there is one answer to "what is this column"
 /// (gotcha 1).
 fn inserts_a_column(ctx: &CmdContext, key: &CellKey) -> bool {
    crate::view::synthetic_record_info(key)
        .and_then(|(_, col_idx)| ctx.layout.record_col(col_idx))
@@ -427,9 +393,8 @@ pub enum AdvanceDir {
    Right,
 }
-/// Return the normal-mode counterpart of an editing mode. Used by
+/// The normal-mode counterpart of an editing mode, for the mode to land in if
-/// `CommitAndAdvance` to compute the mode to land in if the advance
+/// `CommitAndAdvance`'s advance aborts.
 /// aborts (commit + exit editing at boundary).
 fn exit_mode_for(edit_mode: &AppMode) -> AppMode {
    match edit_mode {
        AppMode::RecordsEditing { .. } => AppMode::RecordsNormal,
@@ -437,13 +402,8 @@ fn exit_mode_for(edit_mode: &AppMode) -> AppMode {
    }
 }
-/// Commit a cell edit, advance the cursor, and re-enter edit mode.
+/// Commit a cell edit, advance the cursor, and re-enter `edit_mode` — which the
-/// Subsumes the old `CommitCellEdit` (Down) and `CommitAndAdvanceRight` (Right).
+/// keymap binding supplies. The command never inspects `ctx.mode`.
 ///
 /// `edit_mode` is the editing mode to re-enter after advancing. The keymap
 /// binding supplies this — the editing-mode keymap passes `editing` and the
 /// records-editing keymap passes `records-editing`. The command itself
 /// never inspects `ctx.mode`.
 #[derive(Debug)]
 pub struct CommitAndAdvance {
    pub key: CellKey,
@@ -461,40 +421,13 @@ impl Cmd for CommitAndAdvance {
    }
    fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
        let mut effects: Vec<Box<dyn Effect>> = Vec::new();
-        // Pre-emptively drop to the normal counterpart of edit_mode. If the
+        // Drop to normal **before** the commit, which can abort: tidy-up goes where
-        // advance succeeds, the trailing `EnterEditAtCursor` below will lift
+        // an abort cannot reach it (improvise-2zq, gotcha 21).
        // us back into editing on the new cell. If the advance aborts
        // (e.g. already at bottom-right on Enter), `EnterEditAtCursor` is
        // skipped and we land in normal mode — which is the desired
        // "Enter at bottom-right commits and exits" behavior.
        //
        // It goes **before** the commit, not after (improvise-2zq). The
        // commit can abort the batch itself — `AddCategory` does so when the
        // document is at `MAX_CATEGORIES` — and an abort before this ran
        // left the app in the editing mode, where `draw_bottom_bar` renders
        // the minibuffer instead of the status line and the refusal the
        // effect had just written was never drawn. Tidy-up goes where an
        // abort cannot reach it; same rule as `ExecuteCommand`'s.
        effects.push(effect::change_mode(exit_mode_for(&self.edit_mode)));
        commit_cell_value(ctx, &self.key, &self.value, &mut effects);
        if inserts_a_column(ctx, &self.key) {
-            // A schema change that inserts a column moves every index to its
+            // An inserted column shifts every index right of it, and the advance is
-            // right, and the advance below is computed from `ctx` — the
+            // computed from the *pre-commit* layout (gotcha 23). Staying put lands on it.
            // *pre-commit* layout, because `apply_effects` rebuilds only
            // after the whole batch (gotcha 23). On the last row `Enter`
            // wraps to `col + 1`, which after the insert is `+cat` again, so
            // the editor reopened category creation and a user naming two
            // categories in a row never reached the column they had made.
            // `Tab` reached it the same way.
            //
            // Not advancing is also the *useful* answer rather than merely
            // the safe one: `add_category` inserts the new column exactly
            // where the cursor already is, so staying put lands on it.
            //
            // The measure `+` shares none of this, and not by luck — it is
            // the last column, so there is no column to its right for an
            // index to shift into, and both advances are already special-
            // cased at the bottom-right edge.
            effects.push(Box::new(effect::RebuildLayout));
            return effects;
        }
@@ -544,25 +477,21 @@ impl Cmd for CommitAndAdvance {
    }
 }
-/// What a minibuffer commit produces. The variants differ only in the model
+/// What a minibuffer commit produces. The variants differ only in the mutation they
-/// mutation they build and in where they leave the user — reading the buffer,
+/// build and where they leave the user; the rest is [`CommitBuffer`]'s (gotcha 20).
 /// trimming it, bailing out when it is empty, marking the model dirty and
 /// reporting are shared by [`CommitBuffer`].
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum CommitKind {
    Formula,
    Category,
-    /// Add an item to this category. Resolved from [`AppMode::ItemAdd`] when
+    /// Add an item to this category, resolved from [`AppMode::ItemAdd`] at
-    /// the command is constructed, so `execute` stays a pure function of the
+    /// construction so `execute` stays pure over the kind and its buffer.
    /// kind and its buffer.
    Item {
        category: String,
    },
 }
-/// Resolve a `commit-buffer` argument against the current mode. `Item` is the
+/// Resolve a `commit-buffer` argument against the current mode. Only `Item` needs
-/// only kind that needs anything from the mode — its target category — so a
+/// the mode, so a script line can still build the other two.
 /// script line (which has no live mode) can build the other two.
 pub fn parse_commit_kind(arg: &str, mode: &AppMode) -> Result<CommitKind, String> {
    match (arg, mode) {
        ("formula", _) => Ok(CommitKind::Formula),
@@ -594,9 +523,8 @@ impl CommitKind {
        }
    }
-    /// Where a *successful* commit lands the user. `None` keeps the minibuffer
+    /// Where a *successful* commit lands the user. `None` keeps the minibuffer open:
-    /// open for the next entry — categories and items are entered in batches,
+    /// categories and items are entered in batches, formulas are not.
    /// formulas are not.
    fn on_commit(&self) -> Option<AppMode> {
        match self {
            Self::Formula => Some(AppMode::FormulaPanel),
@@ -629,9 +557,8 @@ impl CommitKind {
    }
 }
-/// Commit the contents of a minibuffer as a formula, a category or an item.
+/// Commit a minibuffer as a formula, a category or an item. Buffer clearing is the
-///
+/// keymap's (Enter → `[commit, clear-buffer]`).
 /// Buffer clearing is handled by the keymap (Enter → [commit, clear-buffer]).
 #[derive(Debug)]
 pub struct CommitBuffer {
    pub kind: CommitKind,
@@ -648,9 +575,8 @@ impl Cmd for CommitBuffer {
        }
        let mut effects: Vec<Box<dyn Effect>> = vec![
            effect::set_status(self.kind.status(name)),
-            // The add goes last on purpose: it is the only party that knows
+            // The add goes last: it is the only party that knows whether it worked,
-            // whether it worked, so the optimistic status above must not
+            // so the optimistic status must not clobber its error (improvise-6t3).
            // clobber the error it reports (improvise-6t3).
            self.kind.add_effect(name),
        ];
        effects.extend(self.kind.on_commit().map(effect::change_mode));
@@ -658,16 +584,8 @@ impl Cmd for CommitBuffer {
    }
 }
-/// Commit a save path from the save buffer.
+/// Commit a save path. The sibling of [`CommitExport`] and deliberately not the same
-///
+/// command — that one writes CSV, this writes the model (improvise-lae).
 /// The sibling of [`CommitExport`], and deliberately not the same command:
 /// the export prompt writes CSV, this one writes the *model* through
 /// `SaveAs`, which also adopts the path so a later plain `Save` has one
 /// (improvise-lae).
 ///
 /// An empty path bails rather than reaching `SaveAs`, which would otherwise
 /// ask the storage backend to write to `""` and report its error instead of
 /// the one the user can act on.
 #[derive(Debug)]
 pub struct CommitSave;
 impl Cmd for CommitSave {
@@ -682,23 +600,8 @@ impl Cmd for CommitSave {
                effect::change_mode(AppMode::Normal),
            ];
        }
-        // The mode change comes **first**, and that ordering is the whole
+        // The mode change comes **first**, or a failed `SaveAs` aborts with the
-        // reason the user sees a failed save (found by Copilot on PR #27).
+        // minibuffer still drawn over its error (Copilot, PR #27).
        //
        // `SaveAs` aborts the batch when the write fails (improvise-8tk's
        // other arm), so anything after it is skipped. With the order
        // reversed the batch stopped while still in `SavePrompt` — and
        // `draw_bottom_bar` renders the minibuffer *instead of* the status
        // line, so the "Save error:" set by the effect was never drawn. The
        // save appeared to do nothing at all.
        //
        // Leading with the mode change means the error lands in `Normal`,
        // where the status bar is what the bottom row draws. It costs the
        // typed path, which the welded `clear-buffer` would have taken on a
        // success anyway and which `EnterMode` clears on the next Ctrl-W
        // regardless (gotcha 11) — so nothing durable is lost, and the
        // convention that the side effect runs last and its message wins is
        // the one gotcha 20 already states.
        vec![
            effect::change_mode(AppMode::Normal),
            Box::new(effect::SaveAs(std::path::PathBuf::from(path))),
@@ -10,11 +10,8 @@ use crate::ui::effect::{Effect, Panel};
 use crate::view::{Axis, GridLayout, View};
 use crate::workbook::Workbook;
-/// Read-only context available to commands for decision-making.
+/// Read-only context available to commands for decision-making: a `&Model`, the
-///
+/// active `&View`, and the `&Workbook` for the rare command that enumerates views.
 /// Commands receive a `&Model` (pure data) and a `&View` (the active view).
 /// The full `&Workbook` is also available for the rare command that needs
 /// to enumerate all views (e.g. the view panel).
 pub struct CmdContext<'a> {
    pub model: &'a Model,
    pub workbook: &'a Workbook,
@@ -118,10 +115,8 @@ pub trait Cmd: Debug + Send + Sync {
 /// Factory that constructs a Cmd from text arguments (headless/script).
 pub type ParseFn = fn(&[String]) -> Result<Box<dyn Cmd>, String>;
-/// Factory that constructs a Cmd from the interactive context (keymap dispatch).
+/// Factory constructing a Cmd from the interactive context. It takes both the keymap
-/// Receives both the keymap args and the interactive context so commands can
+/// args and the context, so a command can combine text args with runtime state.
 /// combine text arguments (e.g. panel name) with runtime state (e.g. whether
 /// the panel is currently open).
 pub type InteractiveFn = fn(&[String], &CmdContext) -> Result<Box<dyn Cmd>, String>;
 type BoxParseFn = Box<dyn Fn(&[String]) -> Result<Box<dyn Cmd>, String>>;
@@ -174,9 +169,8 @@ impl CmdRegistry {
        });
    }
-    /// Register a command that doesn't need interactive context.
+    /// Register a command that needs no interactive context: with args it delegates
-    /// When called interactively with args, delegates to parse.
+    /// to `parse`, and without them it errors.
    /// When called interactively without args, returns an error.
    pub fn register_pure(&mut self, prototype: &dyn Cmd, parse: ParseFn) {
        self.entries.push(CmdEntry {
            name: prototype.name(),
@@ -213,9 +207,8 @@ impl CmdRegistry {
        Err(format!("Unknown command: {name}"))
    }
-    /// Construct a command from interactive context (keymap dispatch).
+    /// Construct a command from interactive context, always with both args and
-    /// Always calls the interactive constructor with both args and ctx,
+    /// `ctx`, so commands can combine text arguments with runtime state.
    /// so commands can combine text arguments with runtime state.
    pub fn interactive(
        &self,
        name: &str,
@@ -269,13 +262,8 @@ pub(super) fn read_buffer(ctx: &CmdContext, name: &str) -> String {
    }
 }
-/// The effect that writes a named buffer — the mirror of [`read_buffer`],
+/// The effect that writes a named buffer — the mirror of [`read_buffer`], and the
-/// and the only place that knows "search" is not an entry in the buffers map
+/// only place that knows "search" is not in the buffers map (improvise-gdl).
 /// but its own `ViewState` field (improvise-gdl).
 ///
 /// The knowledge used to sit inside the `SetBuffer` *effect*, where nothing
 /// paired it with the read side and every caller had to know that passing
 /// `"search"` meant something else than it said.
 pub(super) fn write_buffer(name: &str, value: String) -> Box<dyn Effect> {
    if name == "search" {
        Box::new(crate::ui::effect::SetSearchQuery(value))
@@ -83,8 +83,7 @@ mod tests {
 }
 // ── Parseable model-mutation commands ────────────────────────────────────────
-// These are thin Cmd wrappers around effects, constructible from string args.
+// Thin Cmd wrappers around effects, on the same path as keymap dispatch.
 // They share the same execution path as keymap-dispatched commands.
 macro_rules! effect_cmd {
    ($name:ident, $cmd_name:expr, $parse:expr, $exec:expr) => {
@@ -1,62 +1,5 @@
-//! `fit-measure`: evolve a decision tree over what the active view shows and
+//! `fit-measure`: evolve a decision tree over the active view and store it as an
-//! store it as an ordinary improvise formula.
+//! ordinary `## Formulas` line. The argument is in `context/code/command.md`.
 //!
 //! After this command there is nothing GP-specific left in the document — the
 //! rule is a `## Formulas` line like any other, recomputed by the fixed-point
 //! evaluator, editable in the formula panel, drillable, and saved with no new
 //! grammar (improvise-21d's enabling fact, and improvise-7na's answer).
 //!
 //! # The fit runs inside `execute`, and that is the architecture rather than a
 //! shortcut
 //!
 //! A `Cmd` is pure over `&CmdContext` and returns effects; an `Effect` is the
 //! only mutation site. A fit is **decision logic** — a pure function of
 //! `(Model, View, target, seed, FitConfig)` that mutates nothing — so it
 //! belongs on the command side, and what the effects carry is its *result*.
 //! Putting the search inside an `Effect::apply` would have mixed an expensive
 //! stochastic computation into the one place the app mutates, which is the
 //! split's whole point.
 //!
 //! What this does **not** buy is asynchrony. The command runs on the thread
 //! that draws the next frame, so a fit is bounded rather than cancellable:
 //! [`FitRequest::default`] is a deliberately smaller search than the CLI's
 //! (improvise-vx3's AC 4 asks for "cancellable **or** bounded"). Moving it off
 //! the main loop is a change to `draw.rs`'s event loop, not to this file, and
 //! nothing here forecloses it — the fit is already a pure function that could
 //! be handed to a worker and have its `FitOutcome` posted back as the same
 //! effects.
 //!
 //! # What a refusal is for
 //!
 //! `improvise_gp::fit_measure` fails rather than storing something misleading
 //! — most sharply when the champion splits on an encoding artefact like
 //! `Region=North`, which is not an item of anyone's document, so every cell of
 //! the stored measure would be `Error("ref:…")` (improvise-8xhy, repo-map
 //! gotcha 37d). This command is where a user actually meets that restriction,
 //! so the refusal goes to the status line **whole**: it names the features that
 //! could not be resolved, because "the fit failed" is not something a user can
 //! act on and "it split on Region, which is a category rather than a measure"
 //! is.
 //!
 //! # What registering it costs the browser, measured
 //!
 //! `improvise-command` is in the **worker** bundle, and the path from
 //! `App::new` → `default_keymaps` → `default_registry` → this command's
 //! prototype is live, so `gp-tree` links into it whether or not a browser ever
 //! invokes `fit-measure`. `kb.pl` names keeping GP out of the wasm bundle as
 //! `improvise-gp`'s reason to exist, so this is a cost to state rather than to
 //! discover: built with `wasm-pack --release`, `improvise_worker_server_bg.wasm`
 //! goes **1,341,130 → 1,420,470 bytes** (+79 KB, +5.9%), or **498 → 531 KB
 //! gzipped** (+6.5%). `improvise-wasm-client` — the main-thread bundle — does
 //! not depend on `improvise-command` at all and is **unchanged**.
 //!
 //! Accepted rather than feature-gated, for two reasons. The worker already
 //! carries the whole `App`, persistence and the import wizard, so this is a
 //! 6% addition to something an order of magnitude larger; and a browser fit is
 //! a *wanted* end state, not dead weight — `improvise_gp::fit` exists where it
 //! does precisely so a second host can run the same sequence. If the bundle
 //! ever needs shrinking, a default-on cargo feature that `web/build.sh` turns
 //! off is the lever, and this paragraph is the number to beat.
 use improvise_gp::fit::one_line;
 use improvise_gp::{FitRequest, fit_measure, fitted_measure_name};
@@ -64,11 +7,8 @@ use improvise_gp::{FitRequest, fit_measure, fitted_measure_name};
 use crate::command::cmd::core::{Cmd, CmdContext};
 use crate::ui::effect::{self, Effect};
-/// A seed as the status line spells it (`0x…`, either case) or in decimal.
+/// A seed as the status line spells it (`0x…`, either case) or in decimal. The hex
-///
+/// arm is not a convenience — `status_line` prints `{:#x}`.
 /// The hex arm is not a convenience: `FitOutcome::status_line` prints `{:#x}`
 /// and tells the user that is what reproduces the fit, so accepting only
 /// decimal made the advertised round trip fail on the advertised value.
 fn parse_seed(text: &str) -> Result<u64, String> {
    let parsed = match text.strip_prefix("0x").or_else(|| text.strip_prefix("0X")) {
        Some(hex) => u64::from_str_radix(hex, 16),
@@ -97,16 +37,8 @@ impl FitMeasure {
        }
    }
-    /// `fit-measure <target> [<measure-name>] [<seed>]`.
+    /// `fit-measure <target> [<measure-name>] [<seed>]`. The seed is an argument, not
-    ///
+    /// a clock read, and is read in the form it is printed (gotcha 37b).
    /// The seed is an argument rather than a clock read for the reason the CLI
    /// fixes its own default: two fits of the same document should agree, and a
    /// user who wants a different search asks for one. It is reported back on
    /// the status line so the fit that landed can be asked for again — **which
    /// is why it is read in the form it is printed**, hexadecimal with a `0x`
    /// prefix, as well as decimal. `status_line` spells it `{:#x}`, so a
    /// decimal-only parser made the one instruction the status line gives a
    /// user fail on the value the status line just showed them.
    pub fn parse(args: &[String]) -> Result<Box<dyn Cmd>, String> {
        let [target, rest @ ..] = args else {
            return Err("fit-measure requires a target measure".into());
@@ -140,9 +72,8 @@ impl Cmd for FitMeasure {
    fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
        match fit_measure(ctx.model, ctx.view, &self.target, &self.into, &self.request) {
-            // The report first and `AddFormula` last, so a formula the emitter
+            // The report first and `AddFormula` last, so a formula the parser refuses
-            // spelled but the parser will not take leaves *its* message on
+            // leaves *its* message on screen, not this command's (gotcha 20).
            // screen rather than this command's optimistic one (gotcha 20).
            Ok(outcome) => vec![
                effect::set_status(outcome.status_line()),
                Box::new(effect::AddFormula {
@@ -150,10 +81,8 @@ impl Cmd for FitMeasure {
                    target_category: outcome.formula.target_category.clone(),
                }),
            ],
-            // Flattened like the success line: several refusals interpolate a
+            // Flattened: a refusal interpolates a document name, and a name may
-            // document name, and a name may legally contain a newline
+            // legally contain a newline (Copilot on PR #25).
            // (Copilot on PR #25). A failure is when a user is most reading
            // the status bar, so it is the last place to let one split.
            Err(e) => vec![effect::set_status(one_line(format!("fit-measure: {e}")))],
        }
    }
@@ -171,16 +100,8 @@ mod tests {
    const REGION: &str = "Region";
    const TAG: &str = "Tag";
-    /// A document a tree can learn exactly — `Score` steps at `Cost` 50 — in
+    /// A document a tree can learn exactly. **Every category is a `Label`**, or the
-    /// the units a real document uses.
+    /// champion can reach an unresolvable feature — see the evidence.
    ///
    /// **Every category is a `Label`**, so the only feature is the numeric
    /// measure `Cost` and the champion cannot reach an unresolvable one. An
    /// earlier version left `Region` regular with one item and claimed
    /// `simplify` would prune the constant one-hot; it does not, because
    /// `build_categorical` records `(0.0, 1.0)` for every one-hot whatever the
    /// column holds — so these tests were passing on seed luck (Copilot on
    /// PR #25). See `improvise_gp::fit`'s own fixture for the same correction.
    fn workbook() -> crate::workbook::Workbook {
        let mut wb = crate::workbook::Workbook::new("sales");
        wb.add_category(REGION).expect("first category");
@@ -236,9 +157,8 @@ mod tests {
        app
    }
-    /// The end-to-end claim of improvise-vx3, asserted against a real
+    /// improvise-vx3 end to end against a real document: a command fits, the formula
-    /// document: a command fits, the formula lands through the ordinary
+    /// lands through `add_formula`, and the new measure evaluates to numbers.
    /// `add_formula` path, and the new measure evaluates to numbers.
    #[test]
    fn the_command_installs_a_formula_the_document_evaluates() {
        let app = run(&["Score"]);
@@ -268,10 +188,8 @@ mod tests {
        assert!(run(&["Score"]).model_state.dirty);
    }
-    /// improvise-vx3 is emphatic that the rule alone is not the deliverable:
+    /// The rule alone is not the deliverable, so the status line carries the held-out
-    /// a tidy tree that generalizes no better than the mean is worse than
+    /// comparison, what the rule leans on, and the seed that reproduces it.
    /// useless. So the status line carries the held-out comparison, what the
    /// rule leans on, and the seed that reproduces it.
    #[test]
    fn the_status_line_reports_quality_against_the_baseline() {
        let app = run(&["Score"]);
@@ -284,9 +202,8 @@ mod tests {
        }
    }
-    /// A refusal reaches the user *whole*. "fit failed" is not actionable;
+    /// A refusal reaches the user *whole*: "fit failed" is not actionable, naming the
-    /// naming the measure that does not exist is (improvise-8xhy's argument,
+    /// measure that does not exist is (improvise-8xhy).
    /// arriving at the place a user meets it).
    #[test]
    fn a_refusal_names_what_went_wrong_and_stores_nothing() {
        let wb = workbook();
@@ -306,21 +223,8 @@ mod tests {
        assert!(app.model_state.workbook.model.formulas().is_empty());
    }
-    /// gotcha 37b: the seed is half of what reproduces a fit, so it is an
+    /// gotcha 37b: a different seed really does search differently. It asserts on the
-    /// argument — and a different one really does search differently.
+    /// **rule**, never on the batch debug string — see the evidence.
    ///
    /// It asserts on the **rule**, never on the batch's debug string. The
    /// first version compared `effects_debug` over the whole vec, which was
    /// vacuous: `status_line` prints `seed {:#x}`, so the two strings differ
    /// because the seed was *printed*, and `fit_measure` could have ignored
    /// `request.seed` entirely with the test still green
    /// (`doctrine(check_the_checker)`).
    ///
    /// `AddFormula` has exactly two fields — the formula text and the target
    /// category — and the seed reaches neither, so its `Debug` cannot differ
    /// for the reason the batch's did. (A substring guard was tried and is
    /// wrong: the seeds `1` and `99` occur inside perfectly ordinary
    /// thresholds.)
    #[test]
    fn the_seed_argument_changes_the_search() {
        let wb = workbook();
@@ -350,14 +254,8 @@ mod tests {
        assert!(err.contains("is not a seed"), "{err}");
    }
-    /// Copilot on PR #25. The status line advertises the seed as `{:#x}` and
+    /// Copilot on PR #25: the seed must round-trip *through the status line*, not
-    /// tells the user that is what reproduces the fit, so the one instruction
+    /// against a hand-written `"0x…"` that would survive a respelling.
    /// it gives has to work on the value it just showed them — a decimal-only
    /// parser made copying it back an error.
    ///
    /// Asserted as a *round trip through the status line*, not against a
    /// hand-written `"0x…"`: a literal would still pass if `status_line`
    /// changed how it spells the number, which is the half that breaks.
    #[test]
    fn the_seed_the_status_line_prints_is_a_seed_this_command_accepts() {
        let app = run(&["Score"]);
@@ -120,10 +120,8 @@ mod tests {
        );
    }
-    /// Drilling into a formula cell (e.g. Profit = Revenue - Cost) should
+    /// Drilling into a formula cell must return the underlying data records: the
-    /// return the underlying data records, not an empty result set. The
+    /// target coordinate is stripped so `matching_cells` finds them (gotcha 10).
    /// formula target coordinate is stripped from the drill key so that
    /// matching_cells finds the raw data backing the formula.
    #[test]
    fn drill_into_formula_cell_returns_data_records() {
        use crate::formula::parse_formula;
@@ -172,10 +170,8 @@ mod tests {
        );
    }
-    /// improvise-dz8: a drilled coordinate is a constraint of the drill, not
+    /// improvise-dz8: a drilled coordinate is a constraint, not a dimension to page
-    /// a dimension the user may page through, so it lands on `Filter`. It
+    /// through, so it lands on `Filter`. It used to land on `Page`.
    /// used to land on `Page`, which put it in the tile bar and under
    /// `[`/`]` as if the user could move it.
    #[test]
    fn drilling_fixes_its_coordinates_on_filter_not_page() {
        let m = two_cat_model();
@@ -297,9 +293,8 @@ impl Cmd for ViewNavigate {
    }
 }
-/// Drill down into an aggregated cell: create a _Drill view with _Index on
+/// Drill into an aggregated cell: a `_Drill` view with `_Index` on Row and `_Dim` on
-/// Row and _Dim on Column (records/long-format view). Fixed coordinates
+/// Column. The drilled cell's fixed coordinates become filters (gotcha 27).
 /// from the drilled cell become page filters.
 #[derive(Debug)]
 pub struct DrillIntoCell {
    pub key: crate::model::cell::CellKey,
@@ -312,9 +307,8 @@ impl Cmd for DrillIntoCell {
        let drill_name = crate::view::DRILL_VIEW.to_string();
        let mut effects: Vec<Box<dyn Effect>> = Vec::new();
-        // If drilling into a formula cell, strip the formula target from the
+        // Drilling a formula cell strips the formula target from the key, or
-        // key so matching_cells finds the underlying raw data records instead
+        // `matching_cells` returns nothing instead of the raw records (gotcha 10).
        // of returning nothing.
        let drill_key = if let Some(measure_val) = self.key.get(MEASURE_CATEGORY) {
            let is_formula_target = ctx.model.formulas().iter().any(|f| {
                CategoryKind::of_name(&f.target_category).is_measure() && f.target == measure_val
@@ -365,9 +359,8 @@ impl Cmd for DrillIntoCell {
            axis: crate::view::Axis::Column,
        }));
-        // Fixed coords (from the drilled cell) -> Filter, an immutable
+        // Fixed coords go to Filter: an immutable constraint of this drill rather
-        // constraint of this drill rather than a dimension to page through
+        // than a dimension to page through (improvise-dz8).
        // (improvise-dz8).
        let fixed_cats: std::collections::HashSet<String> =
            self.key.0.iter().map(|(c, _)| c.clone()).collect();
        for (cat, item) in &self.key.0 {
@@ -380,9 +373,8 @@ impl Cmd for DrillIntoCell {
                item: item.clone(),
            }));
        }
-        // Previously-aggregated categories (none_cats) stay on Axis::None so
+        // Previously-aggregated categories stay on `Axis::None` so they do not filter
-        // they don't filter records; they'll appear as columns in records mode.
+        // records; they appear as columns. `_Index`/`_Dim` are already set.
        // Skip virtual categories — we already set _Index/_Dim above.
        for cat in ctx.none_cats() {
            if fixed_cats.contains(cat) || cat.starts_with('_') {
                continue;
@@ -418,9 +410,8 @@ impl Cmd for TogglePruneEmpty {
    }
 }
-/// Toggle between records mode and pivot mode using the view stack.
+/// Toggle records mode through the view stack: entering creates a `_Records` view
-/// Entering records mode creates a `_Records` view and switches to it.
+/// and switches to it, leaving navigates back.
 /// Leaving records mode navigates back to the previous view.
 #[derive(Debug)]
 pub struct ToggleRecordsMode;
 impl Cmd for ToggleRecordsMode {
@@ -431,10 +422,8 @@ impl Cmd for ToggleRecordsMode {
        let is_records = ctx.layout.is_records_mode();
        if is_records {
-            // Leaving records mode: clean up any records with empty CellKeys
+            // Leaving cleans up the empty-`CellKey` records `AddRecordRow` makes with
-            // (produced by AddRecordRow when no page filters are set) before
+            // no page filters — the inverse of the entry `SortData`.
            // restoring the previous view. This is the inverse of `SortData`
            // that runs on entry.
            return vec![
                Box::new(effect::CleanEmptyRecords),
                Box::new(effect::ViewBack),
@@ -37,9 +37,8 @@ mod tests {
        assert_eq!(effects.len(), 2);
    }
-    /// The `enter-export-prompt` shim is gone (improvise-s13); the generic
+    /// The `enter-export-prompt` shim is gone (improvise-s13): the generic
-    /// `enter-mode` carries the mode name instead, and clears the prompt's
+    /// `enter-mode` carries the mode name and clears the prompt's buffer.
    /// buffer on the way in.
    #[test]
    fn enter_mode_export_prompt_sets_mode_and_clears_its_buffer() {
        let m = two_cat_model();
@@ -58,14 +57,8 @@ mod tests {
        );
    }
-    /// improvise-gdl made `write_buffer` the single accessor for a
+    /// improvise-gdl: `EnterMode` built a `SetBuffer` directly, so a "search"-keyed
-    /// runtime-named buffer, because "search" is not an entry in the buffers
+    /// minibuffer would clear a map entry nobody reads. See the evidence.
    /// map but its own `ViewState` field. `EnterMode` built a `SetBuffer`
    /// directly, so a mode whose `buffer_key` is "search" would clear a map
    /// entry nobody reads and leave the previous query on screen — the exact
    /// desync gdl closed. No shipped `AppMode` names that buffer yet, so the
    /// mode here is constructed by hand; that is what makes the test a
    /// demonstration rather than a restatement.
    #[test]
    fn enter_mode_clears_a_search_keyed_minibuffer_through_write_buffer() {
        use crate::ui::app::{MinibufColor, MinibufferConfig};
@@ -129,11 +122,8 @@ mod tests {
        assert!(dbg.contains("Editing"), "Expected Editing mode, got: {dbg}");
    }
-    /// EditOrDrill must trust its `edit_mode` parameter rather than checking
+    /// `EditOrDrill` trusts its `edit_mode` parameter rather than checking
-    /// `ctx.mode` — the records-normal keymap supplies `records-editing`,
+    /// `ctx.mode` — the records-normal keymap supplies `records-editing`.
    /// but the command itself never inspects the runtime mode. This is the
    /// parallel of the (deleted) `enter_edit_mode_produces_editing_mode`
    /// test for the records branch.
    #[test]
    fn edit_or_drill_passes_records_editing_mode_through() {
        let m = two_cat_model();
@@ -153,12 +143,8 @@ mod tests {
        );
    }
-    /// `EnterEditAtCursorCmd` must hand its `target_mode` straight through
+    /// `EnterEditAtCursorCmd` hands `target_mode` straight through, and fronts the
-    /// to the `EnterEditAtCursor` effect — the keymap (records `o` sequence
+    /// pair with `RebuildLayout` so the cursor reads a current layout (improvise-ehz).
    /// or commit-and-advance) decides; the command never inspects ctx.
    /// It fronts the pair with `RebuildLayout` so the cursor is read against
    /// a layout that includes whatever ran earlier in the sequence
    /// (improvise-ehz).
    #[test]
    fn enter_edit_at_cursor_cmd_passes_target_mode_to_effect() {
        let m = two_cat_model();
@@ -232,10 +218,8 @@ impl Cmd for EnterMode {
    }
    fn execute(&self, _ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
        let mut effects: Vec<Box<dyn Effect>> = Vec::new();
-        // Clear the corresponding buffer when entering a text-entry mode.
+        // Clear the mode's buffer through `write_buffer`, the only code that knows a
-        // Through `write_buffer`, which is the only code that knows a
+        // runtime-named buffer may not be in the buffers map (improvise-gdl).
        // runtime-named buffer may not be an entry in the buffers map
        // (improvise-gdl).
        if let Some(mb) = self.0.minibuffer() {
            effects.push(write_buffer(&mb.buffer_key, String::new()));
        }
@@ -273,18 +257,8 @@ impl Cmd for Quit {
    }
 }
-/// Save then quit. The `Quit` only lands if the `Save` succeeds — a failed
+/// Save then quit; the `Quit` only lands if the `Save` succeeds (improvise-8tk).
-/// save aborts the effect chain, so `wq`/`ZZ` leaves the user in the app
+/// **Not an `AAndB` command** — `wq` is the user's name — see `context/code/command.md`.
 /// with the error visible rather than discarding the model (improvise-8tk).
 ///
 /// **This looks like an `AAndB` command and is not one** (improvise-git).
 /// `wq` is the *user's* name for the composition, not an invented internal
 /// one: it is registered, so `:wq` reaches it, and `Quit`'s unsaved-changes
 /// message advertises that spelling. A keymap `Binding::Sequence` cannot be
 /// reached from the `:` line, so replacing this with one would either delete
 /// the ex-command or leave two independently-written spellings of "save then
 /// quit". One definition behind two surfaces is what this already is.
 /// Pinned by `text_buffer::tests::colon_wq_saves_then_quits`.
 #[derive(Debug)]
 pub struct SaveAndQuit;
 impl Cmd for SaveAndQuit {
@@ -298,33 +272,15 @@ impl Cmd for SaveAndQuit {
 // ── Editing entry ───────────────────────────────────────────────────────
-/// Smart dispatch for i/a: if the cursor is on an aggregated pivot cell
+/// Smart dispatch for i/a: drill an aggregated pivot cell, else pre-fill the buffer
-/// (categories on `Axis::None` and the cell is not a synthetic records-mode
+/// and enter `edit_mode` — which the keymap supplies, so this stays mode-agnostic.
 /// row), drill into it instead of editing. Otherwise pre-fill the edit
 /// buffer with the displayed cell value and enter `edit_mode`.
 ///
 /// `edit_mode` is supplied by the keymap binding — the command itself is
 /// mode-agnostic, so the records-normal keymap passes `records-editing`
 /// while the normal keymap passes `editing`.
 #[derive(Debug)]
 pub struct EditOrDrill {
    pub edit_mode: AppMode,
 }
 impl EditOrDrill {
-    /// What to say when there is no cell under the cursor at all.
+    /// What to say when there is no cell under the cursor. It reads `edit_mode`, the
-    ///
+    /// keymap's argument, not `ctx` (improvise-ct5/4ju) — see `context/code/command.md`.
    /// improvise-ct5: this used to say "cannot drill — no cell at cursor" in
    /// both modes. Drilling is a pivot-mode concept, and in records mode —
    /// where every regular category sits on `Axis::None`, so this branch is
    /// the one an empty grid always takes — the user's model is "a table I
    /// type into", with `o` one key away to add a row. Naming drilling there
    /// described a mechanism the user is not using instead of the key that
    /// would help.
    ///
    /// The choice reads `edit_mode` — the argument the *keymap* supplies —
    /// rather than asking `ctx` what mode it is in, so the command stays as
    /// mode-agnostic as its doc comment claims and no runtime `is_records()`
    /// check comes back (improvise-4ju).
    fn nothing_at_cursor(&self) -> &'static str {
        if self.edit_mode.is_records() {
            "No record at cursor — press o to add a row"
@@ -339,9 +295,8 @@ impl Cmd for EditOrDrill {
        "edit-or-drill"
    }
    fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
-        // Only consider regular (non-virtual, non-label) categories on None
+        // Only **regular** categories on None are true aggregation: virtuals like
-        // as true aggregation. Virtuals like _Index/_Dim are always None in
+        // `_Index`/`_Dim` are always None in pivot mode and imply nothing.
        // pivot mode and don't imply aggregation.
        let regular_none = ctx.none_cats().iter().any(|c| {
            ctx.model
                .category(c)
@@ -368,9 +323,8 @@ impl Cmd for EditOrDrill {
    }
 }
-/// Thin command wrapper around the `EnterEditAtCursor` effect so it can
+/// Thin command wrapper around the `EnterEditAtCursor` effect so it can join a
-/// participate in `Binding::Sequence`. `target_mode` is supplied as the
+/// `Binding::Sequence`. `target_mode` is the keymap binding's argument.
 /// command argument by the keymap binding.
 #[derive(Debug)]
 pub struct EnterEditAtCursorCmd {
    pub target_mode: AppMode,
@@ -4,8 +4,7 @@ use crate::view::Axis;
 use super::core::{Cmd, CmdContext};
 // ── Navigation commands ──────────────────────────────────────────────────────
-// All navigation commands take explicit cursor state. The interactive spec
+// All navigation commands take explicit cursor state; see `context/code/command.md`.
 // fills position/bounds from context; the parser accepts them as args.
 /// Shared viewport state for navigation commands.
 #[derive(Debug, Clone, Default)]
@@ -154,9 +153,8 @@ impl Cmd for EnterAdvance {
        } else if c < col_max {
            (0, c + 1)
        } else {
-            // Already at bottom-right — the advance premise no longer holds.
+            // Already at bottom-right: the advance premise no longer holds, so abort
-            // Abort the rest of the batch so the caller's trailing effects
+            // the rest of the batch and the caller's trailing effects are skipped.
            // (e.g. `CommitAndAdvance`'s `EnterEditAtCursor`) are skipped.
            return vec![Box::new(effect::AbortChain)];
        };
        viewport_effects(
@@ -246,11 +244,8 @@ impl Cmd for PagePrev {
    }
 }
-/// Gather (cat_name, items, current_idx) for page-axis categories.
+/// Page-axis categories as `(cat_name, items, current_idx)`. `categories_on(Page)`
-///
+/// and **not** `filtering_cats()`: Filter is not navigable (improvise-dz8).
 /// Deliberately `categories_on(Axis::Page)` and not `filtering_cats()`:
 /// `Filter` coordinates constrain the data but are not navigable, so `[`
 /// and `]` must not page them (improvise-dz8).
 pub(super) fn page_cat_data(ctx: &CmdContext) -> Vec<(String, Vec<String>, usize)> {
    let view = ctx.view;
    let page_cats: Vec<String> = view
@@ -290,9 +285,8 @@ mod tests {
    use super::*;
    use crate::command::cmd::test_helpers::*;
-    /// improvise-dz8: `[` and `]` page through Page-axis categories. A
+    /// improvise-dz8: a coordinate the drill fixed sits on Filter and must be
-    /// coordinate the drill fixed sits on Filter and must be invisible to
+    /// invisible to `[`/`]`, or the user could page out of the drill.
    /// them, or the user could page out of the drill they are inside.
    #[test]
    fn page_cycling_skips_filter_coordinates() {
        let mut m = three_cat_model_with_page();
@@ -367,9 +361,7 @@ mod tests {
        );
    }
-    /// At bottom-right `EnterAdvance` has no place to go, so it emits a
+    /// At bottom-right `EnterAdvance` emits a single `AbortChain`, which is how
    /// single `AbortChain` effect. Trailing effects in a `CommitAndAdvance`
    /// batch (e.g. `EnterEditAtCursor`) are then skipped, which is how
    /// "Enter at bottom-right commits and exits editing" is realised.
    #[test]
    fn enter_advance_at_bottom_right_emits_abort_chain() {
@@ -103,9 +103,8 @@ mod tests {
        );
    }
-    /// Bug improvise-dqn: CyclePanelFocus always picked the first open panel,
+    /// Bug improvise-dqn: `CyclePanelFocus` always picked the first open panel,
-    /// ignoring the currently focused panel. With formula + category open and
+    /// ignoring the currently focused one.
    /// focus on the formula panel, cycling must move to the category panel.
    #[test]
    fn cycle_panel_focus_advances_from_formula_to_category() {
        let m = two_cat_model();
@@ -301,17 +300,8 @@ mod tests {
        assert!(effects.is_empty());
    }
-    /// improvise-aaw asked for this command to be replaced by a keymap
+    /// improvise-aaw: the name is computed from `ctx`, which a literal-arg `Sequence`
-    /// `Binding::Sequence [create-view X, switch-view X, enter-mode normal]`.
+    /// cannot do. Asserted at two view counts — see the evidence.
    /// It cannot be: a `Sequence` carries **literal** args, and the name here
    /// is computed from `ctx` — so a Sequence would hard-code one name and
    /// the second `n` in the view panel would collide with the first.
    ///
    /// The test was called `..._names_incrementally` and asserted only that
    /// `CreateView`/`SwitchView`/`Normal` appeared — it never looked at the
    /// name, which is the one thing that makes this command a command and
    /// not a composition. Now it does, at two different view counts, because
    /// a single count cannot tell "derived from the workbook" from "constant".
    #[test]
    fn create_and_switch_view_names_incrementally() {
        let mut m = two_cat_model();
@@ -410,11 +400,8 @@ impl Cmd for TogglePanelVisibility {
    }
 }
-/// Tab through open panels in order Formula → Category → View, wrapping.
+/// Tab through open panels Formula → Category → View, wrapping, starting from the
-/// The starting point is the panel currently focused (`ctx.mode`); from a
+/// focused one. One open panel self-transitions; none open is a no-op.
 /// non-panel mode (e.g. Normal) focus goes to the first open panel. With a
 /// single open panel this self-transitions back to it; with none open it is
 /// a no-op.
 #[derive(Debug)]
 pub struct CyclePanelFocus {
    pub formula_open: bool,
@@ -601,9 +588,8 @@ impl Cmd for FilterToItem {
    }
 }
-/// Delete the category or item at the panel cursor.
+/// Delete at the panel cursor: a category header deletes the whole category, an item
-/// On a category header -> delete the whole category.
+/// row deletes just that item.
 /// On an item row -> delete just that item.
 #[derive(Debug)]
 pub struct DeleteCategoryAtCursor;
 impl Cmd for DeleteCategoryAtCursor {
@@ -658,19 +644,8 @@ impl Cmd for SwitchViewAtCursor {
    }
 }
-/// Create a new view, switch to it, and return to Normal mode.
+/// Create a new view, switch to it, return to Normal. **Not a shim, despite the
-///
+/// name** (improvise-aaw) — see `context/code/command.md`.
 /// **Not a shim, despite the name** (improvise-aaw). The new view's name is
 /// derived from `ctx` — `format!("View {}", views.len() + 1)` — and a keymap
 /// `Binding::Sequence` can only carry literal args, so the proposed
 /// `[create-view X, switch-view X, enter-mode normal]` would hard-code one
 /// name and the second `n` in the view panel would collide with the first.
 /// The derivation is the decision that makes this a command.
 ///
 /// The ticket's two fallbacks are both behaviour changes wanting a product
 /// decision, not refactors: prompting for a name is a new feature, and
 /// making `create-view` switch by default changes `:create-view` for scripts
 /// and duplicates what `grid.rs` does by hand for `_Drill`/`_Records`.
 #[derive(Debug)]
 pub struct CreateAndSwitchView;
 impl Cmd for CreateAndSwitchView {
@@ -2,11 +2,8 @@ use crate::model::cell::CellKey;
 use crate::ui::app::AppMode;
 use crate::ui::effect::Panel;
-/// Decode a mode-name string (as supplied by `enter-mode`/`edit-or-drill`
+/// Decode a mode-name string into an `AppMode`. The table is `AppMode::from_name` in
-/// keymap bindings) into an `AppMode`.
+/// the protocol crate: the spellings are keymap vocabulary and the keymap is shared.
 /// The table itself is `AppMode::from_name`, beside `AppMode` in the
 /// protocol crate: the spellings are keymap vocabulary and the keymap is
 /// shared, so the browser client decodes `enter-mode` from the same list.
 fn parse_mode_name(s: &str) -> Result<AppMode, String> {
    AppMode::from_name(s).ok_or_else(|| format!("Unknown mode: {s}"))
 }
@@ -525,9 +522,7 @@ pub fn default_registry() -> CmdRegistry {
    r.register_nullary(|| Box::new(CommandModeBackspace));
    // ── Commit ───────────────────────────────────────────────────────────
-    // commit-cell-edit / commit-and-advance-right take a mode-name arg
+    // These take a mode-name arg as args[0]; the command never inspects ctx.mode.
    // (e.g. "editing" or "records-editing") as args[0]. The keymap supplies
    // it; the command never inspects ctx.mode.
    r.register(
        &CommitAndAdvance {
            key: CellKey::new(vec![]),
@@ -651,21 +646,8 @@ mod tests {
        .collect()
    }
-    /// improvise-9cn / improvise-61f: every registration derives its name
+    /// improvise-9cn/61f: four commands name themselves off a *field*, so a wrong
-    /// from a prototype instance, so a typo in the name is impossible. What
+    /// variant compiles and files the command under the wrong name (gotcha 15).
    /// the types do *not* catch is the commands whose `name()` branches on a
    /// field — `ViewNavigate`, `TileAxisOp`, `ToggleGroupAtCursor`,
    /// `CommitAndAdvance`. Their prototype and the value their constructor
    /// builds are the same type, so registering `ViewNavigate { forward:
    /// true }` while building `forward: false` compiles cleanly and files
    /// the command under the wrong name, where dispatch silently misses.
    /// Constructing every entry both ways and comparing names is the only
    /// check that reaches that class.
    ///
    /// Some entries are deliberately one-path — `commit-and-advance-right`
    /// reads the edit buffer and cursor, so its parse fn always errors — but
    /// every entry must be reachable by at least one path, or its name goes
    /// unchecked.
    #[test]
    fn every_entry_constructs_a_command_with_its_registered_name() {
        let r = default_registry();
@@ -706,12 +688,8 @@ mod tests {
        );
    }
-    /// A variant mix-up does not show up as a name mismatch — it shows up as
+    /// A variant mix-up shows up as a **collision**, not a mismatch: the wrong entry
-    /// a collision. Registering `ViewNavigate { forward: false }` where
+    /// takes its sibling's name and the sibling vanishes. Uniqueness catches it.
    /// `forward: true` belongs files a second entry under "view-back" and
    /// removes "view-forward" from the registry entirely; `parse` takes the
    /// first match, so every surviving entry still agrees with its own name.
    /// Uniqueness is what catches it.
    #[test]
    fn registered_names_are_unique() {
        let r = default_registry();
@@ -27,10 +27,8 @@ mod tests {
        assert!(dbg.contains("he"), "Expected 'he' after pop, got: {dbg}");
    }
-    /// Writing the "search" buffer means writing the search query, and
+    /// Writing the "search" buffer means writing the query, and `write_buffer` is the
-    /// `write_buffer` is the one place that knows it (improvise-gdl). Every
+    /// one place that knows it (improvise-gdl).
    /// buffer-writing command routes through it, so `clear-buffer search`
    /// clears the query rather than parking an unread entry in the map.
    #[test]
    fn writing_the_search_buffer_targets_the_search_query() {
        assert!(
@@ -94,21 +92,8 @@ mod tests {
        );
    }
-    /// improvise-git proposed deleting the `SaveAndQuit` command and binding
+    /// improvise-git proposed replacing `SaveAndQuit` with a `Sequence`. `wq` is also
-    /// `ZZ` to a `Binding::Sequence [w, force-quit]` instead — the repo's
+    /// a *registered name* `:wq` reaches — see the evidence for the decline.
    /// "do not name a command AAndB" principle.
    ///
    /// That premise is incomplete, and this test is why. `wq` is not only a
    /// keybinding: it is a **registered command name**, so `:wq` reaches it
    /// through the registry, and `Quit`'s own unsaved-changes message
    /// advertises it ("Use :q! to force quit or :wq to save+quit"). A keymap
    /// `Sequence` is not reachable from the `:` line, so deleting the command
    /// deletes the ex-command with it.
    ///
    /// Keeping the command *and* adding the Sequence would leave two
    /// independently-written spellings of "save then quit", which is the
    /// duplication this codebase keeps curing rather than creating. So this
    /// pins the surface, and the conversion is declined on it.
    #[test]
    fn colon_wq_saves_then_quits() {
        let m = two_cat_model();
@@ -123,17 +108,8 @@ mod tests {
        assert!(dbg.contains("Quit"), "`:wq` must then quit, got: {dbg}");
    }
-    /// improvise-6sq: `ExecuteCommand` appends `change_mode(Normal)` unless
+    /// improvise-6sq: a trailing `ChangeMode(Normal)` undid the edit mode. The
-    /// some effect reports `changes_mode()`. `EnterEditAtCursor` sets
+    /// assertion has to name the snap-back, not just check `changes_mode()`.
    /// `view_state.mode` but did not override it, so
    /// `:enter-edit-at-cursor editing` produced
    /// `[RebuildLayout, EnterEditAtCursor(Editing), ChangeMode(Normal)]` —
    /// the trailing snap-back undid the edit mode and the user never got a
    /// cursor in the minibuffer.
    ///
    /// Asserting on `changes_mode()` alone is not enough here: the appended
    /// `ChangeMode(Normal)` reports `true` itself, so the assertion has to
    /// name the snap-back it is ruling out.
    #[test]
    fn colon_enter_edit_at_cursor_is_not_snapped_back_to_normal() {
        let m = two_cat_model();
@@ -240,11 +216,8 @@ mod tests {
        app.apply_effects(effects);
    }
-    /// improvise-toc: `StartImportWizard` swaps the mode only when the file
+    /// improvise-toc: `StartImportWizard` swaps the mode only on success, so a
-    /// parses, so a per-type `changes_mode()` cannot describe it. Reporting
+    /// per-type answer snapped `:import` straight back out of the wizard.
    /// `false` made `ExecuteCommand` append a return-to-Normal on top of the
    /// success path: `:import d.json` opened the wizard and was immediately
    /// snapped back, leaving the wizard drawn under the Normal keymap.
    #[test]
    fn colon_import_stays_in_the_wizard_on_success() {
        let mut app = wizard_test_app();
@@ -257,9 +230,8 @@ mod tests {
        );
    }
-    /// The other half of the same bug: reporting `true` unconditionally would
+    /// The other half: reporting `true` unconditionally would strand the user in
-    /// strand the user in CommandMode when the import fails. A failed
+    /// `CommandMode` when the import fails.
    /// `:import` must still fall back to Normal.
    #[test]
    fn colon_import_falls_back_to_normal_on_failure() {
        let mut app = wizard_test_app();
@@ -272,10 +244,8 @@ mod tests {
        );
    }
-    /// The `ViewBack` half of improvise-toc: it restores the stored frame's
+    /// The `ViewBack` half of improvise-toc: it restores the stored frame's mode only
-    /// mode only when the back-stack is non-empty. With history present,
+    /// when the back-stack is non-empty, and otherwise falls through to Normal.
    /// `:view-back` must keep the mode it restored; with an empty stack it
    /// falls through to Normal like any other no-op command.
    #[test]
    fn colon_view_back_respects_whether_history_exists() {
        use crate::ui::app::ViewFrame;
@@ -376,9 +346,8 @@ impl Cmd for CommandModeBackspace {
 /// Execute the command in the "command" buffer (the `:` command line).
 #[derive(Debug)]
-/// Execute the `:` command buffer by delegating to the command registry.
+/// Execute the `:` command buffer through the registry. The `:` prompt is another
-/// The `:` prompt is just another frontend to the scripting language —
+/// front end to the scripting language — the same parser as `improvise script`.
 /// same parser as `improvise script`.
 pub struct ExecuteCommand;
 impl Cmd for ExecuteCommand {
    fn name(&self) -> &'static str {
@@ -393,28 +362,8 @@ impl Cmd for ExecuteCommand {
        match crate::command::parse::parse_line_with(ctx.registry, &raw) {
            Ok(cmds) => {
-                // Leave the command line **first**, before anything that can
+                // Leave the command line **first**, before anything that can fail:
-                // fail runs.
+                // an abort must stop the chain, never the bookkeeping (gotcha 21).
                //
                // Still decided at apply time rather than here, because
                // ViewBack, ViewForward and StartImportWizard move the mode
                // only on some runs and no up-front answer is right
                // (improvise-toc). Running it at the head does not weaken
                // that: whatever a later effect sets, it sets *after* this,
                // so the mode a `:` line ends in is unchanged in every case
                // — the only difference is what happens when the batch never
                // reaches the end.
                //
                // Which is the point. A failed `Save`/`SaveAs` sets
                // `abort_effects` (gotcha 21), and an abort at the tail ate
                // this restoration too, stranding the user in `CommandMode`
                // with the stale command line drawn — where
                // `draw_bottom_bar` renders the minibuffer *instead of* the
                // status line, so the error the effect had just set was
                // invisible. The abort must stop the *chain* (`:w bad . q`
                // must not quit) and must not stop the *bookkeeping*, and
                // ordering answers that without giving any effect a "runs
                // anyway" flag: tidy-up goes where an abort cannot reach it.
                let mut effects: Vec<Box<dyn Effect>> = vec![Box::new(
                    effect::ReturnToNormalUnlessMoved(ctx.mode.clone()),
                )];
@@ -100,9 +100,8 @@ impl Cmd for MoveTileCursor {
    }
 }
-/// Cycle or set the axis for the category at the tile cursor.
+/// Cycle or set the axis for the category at the tile cursor, staying in TileSelect.
-/// Stays in TileSelect mode so the user can adjust multiple tiles.
+/// `axis: None` cycles; `axis: Some(a)` sets.
 /// `axis: None` -> cycle, `axis: Some(a)` -> set to `a`.
 #[derive(Debug)]
 pub struct TileAxisOp {
    pub axis: Option<Axis>,
@@ -1,10 +1,5 @@
-//! Import-wizard key handling.
+//! Import-wizard key handling: which key means what depends on the step, and that
-//!
+//! decision is pure, so it lives here (improvise-2wm, gotcha 22).
 //! The wizard is a small step machine, and which key means what depends on
 //! the step it is in. That decision is pure, so it lives here: the command
 //! reads the wizard out of `CmdContext` and emits the discrete effects
 //! (`WizardApply`, `CloseWizard`, `AcceptWizardModel`) that carry it out
 //! (improvise-2wm).
 use crate::command::key::KeyCode;
 use crate::import::wizard::{ImportWizard, WizardStep};
@@ -102,9 +97,8 @@ fn name_model_keys(key: KeyCode) -> Vec<Box<dyn Effect>> {
    }
 }
-/// The confirmation step: the workbook is built and waiting. Enter installs
+/// The confirmation step: Enter installs the built workbook, and both Esc and Left
-/// it, and both Esc and Left return to `NameModel` — unlike every other
+/// return to `NameModel` — Esc here does not discard the import (improvise-907).
 /// step, Esc here does not throw the import away (improvise-907).
 fn done_keys(key: KeyCode) -> Vec<Box<dyn Effect>> {
    match key {
        KeyCode::Enter => vec![Box::new(effect::AcceptWizardModel)],
@@ -123,9 +117,8 @@ fn cursor_keys(key: KeyCode) -> Vec<Box<dyn Effect>> {
    }
 }
-/// Left steps back to the previous step, on every step. It is the one key
+/// Left steps back on every step. It is the one key free everywhere: Backspace edits
-/// free everywhere — Backspace edits the name and the formula buffer, and
+/// buffers and `h` is a printable the name step needs (improvise-907).
 /// `h` is a printable character the name step needs (improvise-907).
 fn back_key(key: KeyCode) -> Vec<Box<dyn Effect>> {
    match key {
        KeyCode::Left => op(WizardOp::Back),
@@ -202,9 +195,8 @@ mod tests {
        assert!(ops(&w, KeyCode::Enter).contains("ConfirmFormula"));
    }
-    /// Enter on the name step builds the workbook into the wizard rather
+    /// Enter on the name step builds the workbook rather than advancing the step
-    /// than advancing the step machine; every other printable key edits the
+    /// machine; every other printable key edits the name.
    /// name.
    #[test]
    fn name_model_enter_builds_the_model() {
        let w = wizard_at(WizardStep::NameModel);
@@ -234,9 +226,8 @@ mod tests {
        }
    }
-    /// On the confirmation step Enter installs the built workbook and Esc
+    /// On the confirmation step Enter installs the built workbook and Esc steps back;
-    /// steps back — it does not throw the import away, and no key closes
+    /// no key closes the wizard outright (improvise-907, from improvise-c6j).
    /// the wizard outright (improvise-907, from improvise-c6j).
    #[test]
    fn done_confirms_on_enter_and_steps_back_on_esc() {
        let w = wizard_at(WizardStep::Done);
@@ -1,14 +1,5 @@
-//! The half of the keymap that needs a command registry.
+//! The half of the keymap that needs a command registry: resolving a bound *name*
-//!
+//! into effects. The **table** is `improvise_protocol::keymap` (gotchas 11-14).
 //! The **table** — `KeyPattern`, `Binding`, `Keymap`, `ModeKey`, `TextEntry`,
 //! `PanelKeymap` and `Keymaps::default_keymaps()` — lives in
 //! `improvise_protocol::keymap`, so the browser client looks up the same
 //! bindings the terminal does rather than mirroring them. Read that module's
 //! docs for why.
 //!
 //! What is here is what a thin client cannot have: resolving a bound *name*
 //! into `Effect`s through a [`CmdRegistry`], and the `SetTransientPrefix`
 //! effect that arms a prefix key.
 use std::sync::Arc;
@@ -22,13 +13,8 @@ pub use improvise_protocol::keymap::{
    Binding, KeyPattern, Keymap, Keymaps, ModeKey, PanelKeymap, TextEntry,
 };
-/// Effect that arms a prefix key, so the next keypress is dispatched against
+/// Effect that arms a prefix key. It records the *address* of the prefix, not the
-/// that prefix's sub-keymap.
+/// sub-keymap: `ViewState` must stay portable (improvise-jc8, gotcha 13).
 ///
 /// It records the *address* of the prefix — the key that opened it — rather
 /// than the sub-keymap itself. `KeymapSet` already owns every keymap, so
 /// holding an `Arc<Keymap>` in `ViewState` would only put a non-portable type
 /// into what is meant to become portable, shared state (improvise-jc8).
 #[derive(Debug)]
 pub struct SetTransientPrefix(pub KeyCode, pub KeyModifiers);
@@ -52,53 +38,8 @@ fn run(
    Some(cmd.execute(ctx))
 }
-/// Turn a binding into effects. The one place a keymap lookup becomes a
+/// Turn a binding into effects. A `Sequence` **flattens** into one batch, and
-/// mutation, and the reason `Binding` itself can be registry-free data.
+/// improvise-0hb's proposal to change that was declined — `context/code/command.md`.
 ///
 /// # Why a `Sequence` flattens, and why it stays that way (improvise-0hb)
 ///
 /// Every step is executed against the **same pre-dispatch `ctx`**, and the
 /// effects are concatenated into one vec that `App::apply_effects` runs as
 /// one batch. So a later *command* cannot observe an earlier one's effects —
 /// only a later *effect* can. improvise-0hb proposed closing that: apply
 /// effects so far between steps, rebuild the layout, build a fresh
 /// `CmdContext`, dispatch the next. The design pass says **no**, on three
 /// findings.
 ///
 /// **It would silently reopen improvise-8tk, a P1 data loss.**
 /// `apply_effects` resets `abort_effects` at entry *and* again when it
 /// breaks, because the flag is per batch. Applying between steps therefore
 /// dispatches step 2 into a fresh batch with the flag cleared, so the `Quit`
 /// after a failed `Save` would land — the exact silent discard 8tk fixed.
 /// Gotcha 21's claim that the abort "carries across sequence steps" is true
 /// *because* of this flattening, not in spite of it. It was also untested
 /// until now: no shipped `Sequence` has a first step that can abort, and
 /// `ZZ` is a prefix onto the single `wq` command.
 /// `app.rs::a_sequence_whose_first_step_aborts_does_not_run_the_second` is
 /// the guard, and it fails when the abort is made per-step.
 ///
 /// **Almost nothing would benefit.** Of the shipped `Sequence` bindings,
 /// all but one come from `TextEntry` and are either `[<commit>, clear-buffer
 /// <buf>]` or `[clear-buffer <buf>, enter-mode <exit>]` — `clear-buffer` and
 /// `enter-mode` take their operand as a literal arg and read nothing from
 /// `ctx`, so a refreshed `ctx` would change no outcome. The single genuine
 /// composition is records `o` = `[add-record-row, enter-edit-at-cursor]`,
 /// and it already works, because `EnterEditAtCursorCmd` fronts its effect
 /// with `RebuildLayout` and the effect re-reads the layout itself (gotcha
 /// 23). Every Esc and every commit keypress would pay a layout rebuild and
 /// a `CmdContext` construction — which recomputes column widths — to buy
 /// that nothing.
 ///
 /// **And the ticket's motivating rewrite does not typecheck as a binding
 /// anyway.** improvise-git's `[(w, []), (force-quit, [])]` cannot dispatch:
 /// `w` is `register_pure`, whose interactive constructor rejects an empty
 /// arg list, so `run` returns `None` and the whole binding is dead. The
 /// nullary spelling is `save`. `every_bound_command_name_is_registered`
 /// checks exactly this (gotcha 15) and would have caught it.
 ///
 /// If command-level transactionality is ever wanted, it wants its own
 /// campaign: the abort must become a property of the *sequence* rather than
 /// of the batch before any step is applied separately.
 fn effects_of(
    binding: &Binding,
    registry: &CmdRegistry,
@@ -430,15 +371,8 @@ mod tests {
        }
    }
-    /// improvise-th7m: `F1` was bound twice on consecutive lines — first to a
+    /// improvise-th7m: `F1` was bound twice and the first was dead. This pins which
-    /// bare `enter-mode` with no args, then to `enter-mode ["help"]`. The
+    /// binding it is meant to carry — the one `?` carries.
    /// first was dead, because `Keymap::bind` *inserts* and the later binding
    /// wins; had it ever been the surviving one it would have failed
    /// `require_args` at dispatch and made `F1` do nothing.
    ///
    /// The risk in deleting a line of a duplicated pair is deleting the wrong
    /// one, so this pins which binding `F1` is meant to carry: the same one
    /// `?` carries, which is what identified it as the intended survivor.
    #[test]
    fn f1_and_question_mark_both_open_help() {
        let ks = KeymapSet::default_keymaps();
@@ -477,16 +411,8 @@ mod tests {
        assert!(search.lookup(KeyCode::Esc, KeyModifiers::NONE).is_some());
    }
-    /// improvise-9cn: `Keymap::dispatch` resolves a binding through
+    /// improvise-9cn/s13: a binding is a silent no-op key in two ways — the name may
-    /// `registry.interactive(name, args, ctx).ok()?`, so a binding is a
+    /// not be registered, *or* the args may not parse. Both walks are here.
    /// silent no-op key in two ways, not one: the name may not be
    /// registered, *or* the args may not parse. Both walks live here.
    ///
    /// The args half matters because improvise-s13 moved bindings out of the
    /// nullary class the name check covers: `enter-mode ["export-prompt"]`
    /// and `enter-mode ["formula-edit"]` carry a mode name that
    /// `parse_mode_name` must accept, and a typo like `"export_prompt"`
    /// compiles, registers, and produces a dead `Ctrl-e`.
    #[test]
    fn every_bound_command_name_is_registered() {
        let ks = KeymapSet::default_keymaps();
@@ -496,9 +422,8 @@ mod tests {
        let mut checked = 0usize;
        for (mode, km) in ks.maps().iter() {
-            // Build the context in the mode whose keymap this is: a few
+            // Build the context in the mode whose keymap this is: `commit-buffer
-            // parsers read `ctx.mode` (`commit-buffer item` resolves its
+            // item` resolves its target category off `AppMode::ItemAdd`.
            // target category off `AppMode::ItemAdd`).
            let app_mode = app_mode_for(mode);
            let mut ctx = test_helpers::make_ctx(&wb, &layout, ks.registry());
            ctx.mode = &app_mode;
@@ -538,17 +463,8 @@ mod tests {
        assert!(checked > 100, "expected to walk the keymaps, saw {checked}");
    }
-    /// improvise-2hi: `AppMode::hint_text` is the status bar's promise about
+    /// improvise-2hi: every `Key:verb` token in every hint must resolve in that
-    /// what the keys do, and nothing tied it to the keymaps.
+    /// mode's keymap. Which keys are named, never what they do — see the evidence.
    /// `only_quit_has_no_hint` asserts a hint is *present*, never that it is
    /// *accurate*, so rebinding or dropping a key leaves the hint quietly
    /// lying to the user. This walks every `Key:verb` token in every hint and
    /// requires the key to resolve in that mode's keymap.
    ///
    /// The hint is the source of truth for *which* keys are worth mentioning;
    /// the keymap is the source of truth for what they do. Only the first
    /// half is checkable from here — a hint that says `i:edit` where `i`
    /// enters records mode still passes.
    #[test]
    fn every_hint_key_resolves_in_its_mode() {
        let ks = KeymapSet::default_keymaps();
@@ -571,9 +487,8 @@ mod tests {
        assert!(checked > 50, "expected to walk the hints, saw {checked}");
    }
-    /// Every key a hint mentions. Anything it cannot classify is a panic,
+    /// Every key a hint mentions. An unclassifiable token is a **panic, not a skip**:
-    /// not a skip: a hint token this does not understand is exactly the
+    /// that is exactly the token that would go unchecked.
    /// token that would go unchecked.
    fn hint_keys(hint: &str) -> Vec<(KeyCode, KeyModifiers)> {
        // Everything after the em dash is prose ("— type a category name").
        let head = hint.split('—').next().unwrap_or_default();
@@ -588,9 +503,8 @@ mod tests {
                    keys.extend(hint_key_spec(spec, hint));
                    seen_hint = true;
                }
-                // A colon-less word continues the previous hint's verb
+                // A colon-less word continues the previous hint's verb; one with
-                // ("Enter:add & continue", ":q quit"). One with nothing to
+                // nothing to continue is a token this test does not understand.
                // continue is a token this test does not understand.
                None => assert!(
                    seen_hint,
                    "hint token {token:?} in {hint:?} is not a Key:verb hint"
@@ -679,12 +593,8 @@ mod tests {
        keys
    }
-    /// improvise-jc8: an armed prefix is stored as the key that opened it and
+    /// improvise-jc8: an armed prefix resolves against the current mode's keymap, so
-    /// resolved against the CURRENT MODE's keymap. That is only correct while
+    /// a nested prefix would silently do nothing. Fail here, not in the terminal.
    /// prefixes are flat — a `Prefix` binding nested inside a sub-keymap
    /// would be resolved against the mode map, where it does not exist, and
    /// the second half of the sequence would silently do nothing. Adding a
    /// nested prefix must fail here rather than in the terminal.
    #[test]
    fn prefix_keymaps_are_flat() {
        let ks = KeymapSet::default_keymaps();
@@ -739,10 +649,8 @@ mod tests {
        }
    }
-    /// improvise-4pg: the three panel keymaps were written out separately, so
+    /// improvise-4pg: the three panel keymaps share `PanelKeymap` now; this pins what
-    /// a spine binding could be changed in one and forgotten in the others.
+    /// "the spine" is, each panel's cursor keys included.
    /// They share `PanelKeymap` now; this pins what "the spine" is, including
    /// that each panel's cursor keys move its *own* cursor.
    #[test]
    fn every_panel_keymap_shares_the_same_spine() {
        let ks = KeymapSet::default_keymaps();
@@ -779,9 +687,8 @@ mod tests {
        }
    }
-    /// Every panel's toggle key is bound in Normal and in all three panel
+    /// Every panel's toggle key is bound in Normal and in all three panel modes. A
-    /// modes, so focus can move between any two and the focused one closes.
+    /// fourth panel gets all four keymaps from one `Panel::ALL` entry.
    /// A fourth panel gets all four keymaps from one `Panel::ALL` entry.
    #[test]
    fn every_mode_with_panel_toggles_binds_all_of_them() {
        let ks = KeymapSet::default_keymaps();
@@ -808,10 +715,8 @@ mod tests {
        }
    }
-    /// `Panel::name()` has to spell what the registry's panel parser accepts.
+    /// `Panel::name()` must spell what the registry's parser accepts, and `dispatch`
-    /// A lookup-only assertion would not catch a mismatch — `dispatch`
+    /// swallows a mismatch with `.ok()?` — so real effects must come back.
    /// swallows an unresolvable command with `.ok()?` — so the toggle key
    /// must come back with real effects.
    #[test]
    fn panel_toggle_keys_dispatch_to_real_effects() {
        use crate::command::cmd::test_helpers::*;
@@ -837,10 +742,8 @@ mod tests {
        }
    }
-    /// The seven modes that carry a `MinibufferConfig`, each with the keys
+    /// The seven modes carrying a `MinibufferConfig`, with their commit/leave keys.
-    /// that commit or leave it. `SearchMode` is not among them: it is
+    /// `SearchMode` is Normal plus a flag, so it has no config to agree with.
    /// `Normal` plus a flag, so it has no `AppMode` variant and no
    /// minibuffer config to agree with.
    fn text_entry_modes_with_exits() -> Vec<(AppMode, &'static [KeyCode])> {
        const COMMIT_STAY: &[KeyCode] = &[KeyCode::Esc, KeyCode::Enter, KeyCode::Tab];
        const COMMIT_LEAVE: &[KeyCode] = &[KeyCode::Esc, KeyCode::Enter];
@@ -876,12 +779,8 @@ mod tests {
        }
    }
-    /// improvise-k8h: the buffer name used to be written on both sides of the
+    /// improvise-k8h: the prompt's buffer and the buffer `append-char` edits used to
-    /// crate — `AppMode`'s `MinibufferConfig` names the buffer the prompt
+    /// be written separately, and could disagree with nothing to notice.
    /// describes, and the keymap names the buffer `append-char` edits. If
    /// those disagree the mode prompts over one buffer while every keystroke
    /// edits another, with nothing to notice. `TextEntry` now reads the name
    /// off the mode, and this pins the result for every text-entry mode.
    #[test]
    fn every_text_entry_mode_types_into_the_buffer_its_prompt_names() {
        let ks = KeymapSet::default_keymaps();
@@ -899,9 +798,8 @@ mod tests {
        }
    }
-    /// The list above must stay complete: every keymap that types into a
+    /// The list above must stay complete: every keymap that types into a buffer
-    /// buffer belongs to a mode whose minibuffer names that same buffer.
+    /// belongs to a text-entry mode. Only `SearchMode` is exempt.
    /// Only `SearchMode`, which has no `AppMode` variant, is exempt.
    #[test]
    fn every_typing_keymap_belongs_to_a_text_entry_mode() {
        let ks = KeymapSet::default_keymaps();
@@ -929,18 +827,14 @@ mod tests {
        }
    }
-    /// Every minibuffer mode types into exactly one buffer, and every key that
+    /// Every commit or leave key clears the buffer it typed into — one left behind
-    /// commits or leaves must clear that same buffer on the way — a buffer left
+    /// pre-fills the next entry. `SearchMode` is excluded on purpose.
    /// behind pre-fills the next entry with the last one. `SearchMode` is
    /// excluded on purpose: it has no commit and its query is owned by
    /// `exit-search-mode`, not by `clear-buffer`.
    #[test]
    fn text_entry_modes_clear_the_buffer_they_type_into() {
        let none = KeyModifiers::NONE;
        let ks = KeymapSet::default_keymaps();
-        // The buffer is derived from each mode rather than restated here —
+        // The buffer is derived from each mode rather than restated here; pinning
-        // `every_text_entry_mode_types_into_the_buffer_its_prompt_names`
+        // what it should be is k8h's test above.
        // owns pinning what it should be (improvise-k8h).
        for (app_mode, exits) in text_entry_modes_with_exits() {
            let mode = ModeKey::from_app_mode(&app_mode, false).unwrap();
            let buffer = append_target(&ks, &app_mode);
@@ -960,10 +854,8 @@ mod tests {
        }
    }
-    /// improvise-6t3: the three commit commands collapsed into a single
+    /// improvise-6t3: `dispatch` swallows an unresolvable name with `.ok()?`, so a
-    /// `commit-buffer <kind>`. `dispatch` swallows an unresolvable command name
+    /// lookup-only assertion would pass had the rename missed the registry.
    /// with `.ok()?`, so a lookup-only assertion would still pass had the
    /// rename missed the registry — Enter has to come back with real effects.
    #[test]
    fn minibuffer_enter_dispatches_to_a_registered_commit() {
        use crate::command::cmd::test_helpers::*;
@@ -1018,19 +910,8 @@ mod binding_dump {
    use super::tests_support::*;
    use super::*;
-    /// The instrument `doctrine(binding_dump_diff)` asks for: every
+    /// `doctrine(binding_dump_diff)`'s instrument: every lookup in every mode, to the
-    /// `(mode, key, modifier)` lookup across all sixteen modes, written to
+    /// file `IMPROVISE_BINDING_DUMP` names. Inert without it — see the evidence.
    /// the file named by `IMPROVISE_BINDING_DUMP`.
    ///
    /// Most bindings have no direct test, so a green suite is not evidence
    /// that a keymap refactor preserved them. Dump before, dump after, diff.
    /// It is inert without the variable: a test that writes a file nobody
    /// asked for is a surprise, not a check.
    ///
    /// This is what proved the keymap move to `improvise-protocol` changed
    /// nothing — 4021 resolved lookups, byte-identical either side — and
    /// the diff was shown to be discriminating by renaming one binding and
    /// watching eight lines move.
    #[test]
    fn dump_every_binding() {
        let Ok(path) = std::env::var("IMPROVISE_BINDING_DUMP") else {
@@ -1,14 +1,9 @@
-//! Command layer — all model mutations go through this layer so they can be
+//! Command layer — every model mutation goes through it, so mutations can be
-//! replayed, scripted, and tested without the TUI.
+//! replayed, scripted and tested without the TUI. See `context/code/command.md`.
 //!
 //! Commands are trait objects (`dyn Cmd`) that produce effects (`dyn Effect`).
 //! The headless CLI (--cmd / --script) parses quasi-lisp text into effects
 //! and applies them directly.
 pub mod cmd;
-/// Key types moved to `improvise-protocol` (improvise-cqi) — they ride in
+/// Key types live in `improvise-protocol` (improvise-cqi) — they ride in
-/// `ViewState::transient_prefix`, so they must live where the wire types do.
+/// `ViewState::transient_prefix`. Re-exported so existing paths keep working.
 /// Re-exported here so `crate::command::key::…` paths keep working.
 pub mod key {
    pub use improvise_protocol::key::*;
 }
@@ -1,9 +1,5 @@
-//! Quasi-lisp prefix command parser.
+//! Quasi-lisp prefix command parser: `word arg1 arg2 …`, `.` between commands,
-//!
+//! `Category/Item` coordinate pairs, `"quoted strings"`.
 //! Syntax: `word arg1 arg2 ...`
 //! Multiple commands on one line separated by `.`
 //! Coordinate pairs use `/`: `Category/Item`
 //! Quoted strings supported: `"Profit = Revenue - Cost"`
 use super::cmd::{Cmd, CmdRegistry, default_registry};
@@ -1,7 +1,5 @@
-//! Command layer and semantic app state for improvise — no terminal deps.
+//! Command layer and semantic app state for improvise — no terminal deps. The
-//!
+//! `Cmd`/`Effect` pipeline, keymaps, and the `App` state machine.
 //! Holds the `Cmd`/`Effect` pipeline, keymaps, and the `App` state machine.
 //! Rendering lives in the root `improvise` crate.
 pub mod command;
 pub mod ui;
@@ -12,9 +10,6 @@ pub use improvise_core::{format, model, view, workbook};
 pub use improvise_formula as formula;
 pub use improvise_io::{import, persistence, storage};
-/// The wire vocabulary and the per-widget caches (improvise-rkeq).
+/// The wire vocabulary and the per-widget caches (improvise-rkeq), re-exported so a
-///
+/// host can name a cache without its own `improvise-protocol` dependency.
 /// Re-exported so a host — the ratatui binary, a wasm worker — can name a
 /// widget cache without taking its own `improvise-protocol` dependency, and
 /// so the cache types a host renders are the same ones `ui::project` builds.
 pub use improvise_protocol as protocol;
@@ -1,10 +1,5 @@
-//! The category panel's flattening.
+//! The category panel's flattening. The derivation itself is
-//!
+//! `improvise_protocol::viewmodel` (improvise-edp); what is left here is the path.
 //! The derivation itself moved to `improvise_protocol::viewmodel`
 //! (improvise-edp), because the browser client needs it and can only reach
 //! the protocol crate. What is left here is the path that call sites in this
 //! crate and in the TUI already spell, and the one convenience that needs an
 //! `App`: building the cache before flattening it.
 pub use improvise_protocol::viewmodel::{CatTreeEntry, flatten_cat_tree};
@@ -13,9 +8,8 @@ use std::collections::HashSet;
 use crate::model::Model;
 use crate::view::View;
-/// Build the flattened tree of categories and their items straight from the
+/// Build the flattened tree straight from the model, for a host that holds the
-/// model, for a host that holds the authoritative state rather than a folded
+/// authoritative state — the shortcut `project::category_tree_of` is for widgets.
 /// cache — the same shortcut `project::category_tree_of` is for the widgets.
 pub fn build_cat_tree(model: &Model, view: &View, expanded: &HashSet<String>) -> Vec<CatTreeEntry> {
    let cache = improvise_protocol::CategoryTreeCache {
        categories: crate::ui::project::category_rows(model, view),
@@ -48,12 +42,8 @@ mod tests {
        names.iter().map(|n| n.to_string()).collect()
    }
-    /// improvise-vjms: the panel lists the virtual categories alongside the
+    /// improvise-vjms: the panel lists the virtual categories, and `_Measure`'s items
-    /// regular ones, and `_Measure`'s items are *derived* — explicit items ∪
+    /// are *derived* (gotcha 7). Reading `.items` passes every other assertion here.
    /// formula targets (gotcha 7). An implementation reading
    /// `category("_Measure").items` passes every other assertion here and
    /// silently drops `Profit`, which is the failure improvise-ete was
    /// closed for proposing.
    #[test]
    fn the_tree_carries_the_virtual_categories_and_measure_includes_formula_targets() {
        let mut wb = workbook_with_categories(&[("Region", &["North", "South"])]);
@@ -6,9 +6,8 @@ use crate::view::{Axis, RecordCol, RecordRow};
 use super::app::{App, AppMode, ViewFrame};
-/// `Panel` is defined next to `AppMode` (the two are inverses of each other:
+/// `Panel` is defined next to `AppMode` (`Panel::mode` / `AppMode::panel` are
-/// `Panel::mode` / `AppMode::panel`) and re-exported here, where most callers
+/// inverses) and re-exported here, where most callers already look for it.
 /// already look for it.
 pub use super::app::Panel;
 pub(crate) const RECORD_COORDS_CANNOT_BE_EMPTY: &str = "Record coordinates cannot be empty";
@@ -17,12 +16,8 @@ pub(crate) const RECORD_COORDS_CANNOT_BE_EMPTY: &str = "Record coordinates canno
 /// have to be told about it (improvise-mae).
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum EffectKind {
-    /// Changes only the acting client's own view — cursor, scroll, mode,
+    /// Changes only the acting client's own view — cursor, scroll, mode, panels,
-    /// panels, minibuffer text, drill staging, status line. Never projected.
+    /// minibuffer, drill staging, status line. Never projected (improvise-8uz).
    ///
    /// Cursor and scroll live in the shared `View` (they are persisted in
    /// `.improv`), but they are a per-client concern: one client scrolling
    /// must not move another's viewport. See improvise-8uz.
    ViewOnly,
    /// Changes something every client renders.
    Model(ModelChange),
@@ -46,33 +41,13 @@ pub enum ModelChange {
 pub trait Effect: Debug {
    fn apply(&self, app: &mut App);
-    /// How this effect must be projected to other clients. Deliberately has
+    /// How this effect must be projected. **No default**: a new effect that
-    /// no default: a new effect that forgets to classify itself would
+    /// forgot to classify itself would silently reach no client (improvise-mae).
    /// silently never reach connected clients, so the compiler asks
    /// (improvise-mae).
    fn kind(&self) -> EffectKind;
 }
-/// Declare a setter effect: a one-field newtype whose `apply` writes that
+/// A one-field newtype whose `apply` writes that field into one place on the
-/// field into one place on the `App`.
+/// `App`. The kind argument is mandatory — see `context/code/command.md`.
 ///
 /// The kind is a **mandatory** argument, even though every call site so far
 /// answers `ViewOnly`. Hard-coding it reintroduced exactly the default that
 /// `Effect::kind` deliberately does without (gotcha 3): `$place` is an
 /// unconstrained expression, so
 /// `simple_effect!(X(String) => |app| app.model_state.workbook.active_view_mut().number_format, …)`
 /// is a textbook `LayoutGeometry` write that the macro would have reported as
 /// `ViewOnly`, silently, with the whole suite green. Writing the answer per
 /// call site is the same reasoning that gave `kind()` no default.
 ///
 /// Within that, the macro is still the single hook for anything these setters
 /// should all do — today none of them marks the document dirty or rebuilds the
 /// layout, and the macro makes that one decision instead of fifteen silent
 /// ones (improvise-8fy).
 ///
 /// `Clone::clone` rather than `.clone()`: the field is `usize` or `bool` as
 /// often as it is a `String`, and the call form keeps `clippy::clone_on_copy`
 /// off the generated code.
 macro_rules! simple_effect {
    ($(#[$meta:meta])* $name:ident($ty:ty) => |$app:ident| $place:expr, $kind:expr) => {
        $(#[$meta])*
@@ -95,17 +70,8 @@ macro_rules! simple_effect {
 pub struct AddCategory(pub String);
 impl Effect for AddCategory {
    fn apply(&self, app: &mut App) {
-        // A refusal aborts the batch, for the same reason a failed save does
+        // A refusal aborts the batch for gotcha 21's reason: what follows
-        // (gotcha 21): what follows assumes the category exists. From the
+        // assumes the category exists (improvise-2zq).
        // `+cat` column (improvise-2zq) the rest of the batch would advance
        // the cursor and re-open an editor on a column that was never
        // created — and re-opening the minibuffer is also what would *hide*
        // this message, since `draw_bottom_bar` renders the minibuffer
        // instead of the status line.
        //
        // `MAX_CATEGORIES` is a reachable refusal on that path rather than a
        // theoretical one, and the limit is checked in exactly one place —
        // `Model::add_category` — so nothing here restates it.
        if let Err(e) = app.model_state.workbook.add_category(&self.0) {
            app.view_state.status_msg = e.to_string();
            app.abort_effects = true;
@@ -202,9 +168,8 @@ impl Effect for AddFormula {
    fn apply(&self, app: &mut App) {
        match crate::formula::parse_formula(&self.raw, &self.target_category) {
            Ok(formula) => {
-                // For non-_Measure targets, add the item to the category so it
+                // Non-_Measure targets need the item added so it appears in the
-                // appears in the grid. _Measure targets are dynamically included
+                // grid; _Measure targets arrive via `measure_item_names()`.
                // via Model::measure_item_names().
                if !crate::model::category::CategoryKind::of_name(&formula.target_category)
                    .is_measure()
                {
@@ -242,9 +207,8 @@ impl Effect for RemoveFormula {
    }
 }
-/// Refresh the derived grid layout mid-batch. `apply_effects` rebuilds once
+/// Refresh the derived grid layout mid-batch: `apply_effects` rebuilds only once
-/// after the whole batch, so any effect that must *read* the layout after an
+/// after the batch, so an effect that *reads* it needs this first (improvise-ehz).
 /// earlier effect changed it needs this in front of it (improvise-ehz).
 #[derive(Debug)]
 pub struct RebuildLayout;
 impl Effect for RebuildLayout {
@@ -256,16 +220,8 @@ impl Effect for RebuildLayout {
    }
 }
-/// Re-enter edit mode by reading the cell value at the current cursor.
+/// Re-enter edit mode from the cell at the cursor. `target_mode` is the caller's
-/// Used after commit+advance to continue data entry.
+/// decision, never `app.view_state.mode` — see `context/code/command.md`.
 ///
 /// Reads the *current* layout — emit `RebuildLayout` first when an earlier
 /// effect in the batch moved the cursor or changed the grid.
 ///
 /// `target_mode` is supplied by the caller (keymap binding via
 /// `EnterEditAtCursorCmd`, or `CommitAndAdvance` from its own `edit_mode`
 /// field). The effect itself never inspects `app.view_state.mode` — the mode is decided
 /// statically by whoever invoked us.
 #[derive(Debug)]
 pub struct EnterEditAtCursor {
    pub target_mode: AppMode,
@@ -506,9 +462,8 @@ pub struct TransposeAxes;
 impl Effect for TransposeAxes {
    fn apply(&self, app: &mut App) {
        app.model_state.workbook.active_view_mut().transpose_axes();
-        // Swapping the axes renumbers every row and column, so the old
+        // Swapping the axes renumbers every row and column, so the old indices
-        // indices name different cells. The reset used to live inside
+        // name different cells (improvise-8uz, gotcha 24).
        // View::transpose_axes; it moved here with the cursor (improvise-8uz).
        app.reset_cursor();
    }
    fn kind(&self) -> EffectKind {
@@ -565,16 +520,8 @@ simple_effect!(SetColOffset(usize) => |app| app.cursor_mut().col_offset, EffectK
 simple_effect!(ChangeMode(AppMode) => |app| app.view_state.mode, EffectKind::ViewOnly);
-/// Return to `Normal` unless something earlier in this batch already moved
+/// Return to `Normal` unless something earlier in this batch already moved the
-/// the mode away from `from` — the mode the batch started in.
+/// mode away from `from`. Observed, never predicted (improvise-toc, gotcha 31).
 ///
 /// `ExecuteCommand` used to decide this up front by asking each effect a
 /// per-type `changes_mode()`. That cannot describe `ViewBack`, `ViewForward`
 /// or `StartImportWizard`, which swap the mode only on some runs: answering
 /// "no" snapped a successful `:import` straight back out of the wizard,
 /// while answering "yes" would have stranded a failed one in CommandMode.
 /// Reading the mode here, after the batch has run, needs no prediction
 /// (improvise-toc).
 #[derive(Debug)]
 pub struct ReturnToNormalUnlessMoved(pub AppMode);
 impl Effect for ReturnToNormalUnlessMoved {
@@ -596,12 +543,8 @@ simple_effect!(SetSearchQuery(String) => |app| app.view_state.search_query, Effe
 simple_effect!(SetSearchMode(bool) => |app| app.view_state.search_mode, EffectKind::ViewOnly);
-/// Set a named buffer's contents.
+/// Set a named buffer's contents. The search query is *not* a buffer; commands
-///
+/// pick between the two in `cmd::core::write_buffer` (improvise-gdl).
 /// Every name goes into the buffers map. The search query is not a buffer —
 /// it has its own `ViewState` field and its own `SetSearchQuery` effect — and
 /// commands reach the right one through `cmd::core::write_buffer`, next to
 /// the `read_buffer` it mirrors (improvise-gdl).
 #[derive(Debug)]
 pub struct SetBuffer {
    pub name: String,
@@ -658,17 +601,13 @@ impl Effect for ApplyAndClearDrill {
                RecordCol::Category(category) => {
                    apply_coordinate_rename(app, row, category, new_value);
                }
-                // Never staged — `commit_cell_value` applies a new measure
+                // Never staged: `commit_cell_value` applies a new measure or
-                // or category immediately, since a schema change is not a
+                // category immediately — a schema change is not a cell edit.
                // cell edit and has nothing to roll back with the drill.
                RecordCol::NewMeasure | RecordCol::NewCategory => {}
            }
        }
        // No `dirty = true` here: this declares `Model(CellData)` and
-        // `App::apply_effects` raises the flag off that (improvise-gv9r). The
+        // `App::apply_effects` raises the flag off that (improvise-gv9r).
        // line that used to be here was the last survivor of `MarkDirty` —
        // harmless, and a second home for the one thing that commit set out
        // to give exactly one.
    }
    fn kind(&self) -> EffectKind {
        EffectKind::Model(ModelChange::CellData)
@@ -754,9 +693,8 @@ impl Effect for Save {
                app.model_state.dirty = false;
                app.view_state.status_msg = msg;
            }
-            // A failed save aborts the rest of the batch, so the `Quit`
+            // A failed save aborts the rest of the batch, so the `Quit` that
-            // that `wq`/`ZZ` chains after this never lands and the error
+            // `wq`/`ZZ` chains never lands (improvise-8tk, gotcha 21).
            // stays on screen (improvise-8tk).
            Err(msg) => {
                app.view_state.status_msg = msg;
                app.abort_effects = true;
@@ -794,9 +732,8 @@ impl Effect for SaveAs {
                app.model_state.dirty = false;
                app.view_state.status_msg = format!("Saved to {}", self.0.display());
            }
-            // The same abort `Save` owes (improvise-8tk). `SaveAs` is the
+            // The same abort `Save` owes: `SaveAs` is the arm improvise-8tk
-            // other arm of that rule and was left out of the original fix,
+            // left out, so `:w /bad/path . q` quit on a failed write.
            // so `:w /bad/path . q` quit on a failed write.
            Err(e) => {
                app.view_state.status_msg = format!("Save error: {e}");
                app.abort_effects = true;
@@ -808,9 +745,8 @@ impl Effect for SaveAs {
    }
 }
-/// One discrete mutation of the import wizard's own state. Which op a key
+/// One discrete mutation of the import wizard's own state. Which op a key means
-/// means in a given step is decided by `HandleWizardKey`; this enum is just
+/// in a given step is `HandleWizardKey`'s call; this is just the verb (improvise-2wm).
 /// the verb (improvise-2wm).
 #[derive(Debug, Clone, PartialEq)]
 pub enum WizardOp {
    Advance,
@@ -881,9 +817,8 @@ impl Effect for CloseWizard {
    }
 }
-/// Commit the wizard: build a workbook from its proposals and install it.
+/// Commit the wizard: build a workbook from its proposals and install it. On
-/// On failure the wizard stays open carrying the error so the user can go
+/// failure the wizard stays open carrying the error, so the user can go back.
 /// back and fix their field choices.
 #[derive(Debug)]
 pub struct AcceptWizardModel;
 impl Effect for AcceptWizardModel {
@@ -978,10 +913,8 @@ impl Effect for LoadModel {
    }
 }
-/// Extract the records array from an already-parsed JSON import value.
+/// The records array out of an already-parsed JSON import value: explicit
-/// Precedence: explicit `array_path` → root array → first auto-detected
+/// `array_path` → root array → first auto-detected. Pure (improvise-oaq).
 /// array path. Pure — lets [`ImportJsonHeadless`] parse the file exactly
 /// once and reuse the parsed value as the pipeline's `raw` (improvise-oaq).
 fn json_import_records(
    value: &serde_json::Value,
    array_path: Option<&str>,
@@ -1022,9 +955,8 @@ impl Effect for ImportJsonHeadless {
            .extension()
            .is_some_and(|ext| ext.eq_ignore_ascii_case("csv"));
-        // Parse the file exactly once: `records` feeds analysis, `raw` is the
+        // Parse the file exactly once: `records` feeds analysis, `raw` is reused
-        // parsed value reused for the pipeline (improvise-oaq). CSV warnings
+        // as the pipeline's value (improvise-oaq). CSV warnings go to the status.
        // (short rows, improvise-k8i) are threaded into the status message.
        let mut warnings: Vec<String> = vec![];
        let (records, raw) = if is_csv {
            let bytes = match app.storage.read(&self.path) {
@@ -1152,11 +1084,8 @@ impl Effect for SetPanelCursor {
 // ── Chain control ────────────────────────────────────────────────────────────
-/// Signals `App::apply_effects` to skip the remaining effects in the batch.
+/// Skip the remaining effects in this `apply_effects` batch, for when a
-/// The flag is reset at the start of every `apply_effects` call, so each
+/// sequence's premise no longer holds. The flag resets per dispatch.
 /// dispatch starts clean. Use this when a sequence's premise no longer
 /// holds (e.g. "advance to next cell" at bottom-right) and later effects
 /// (e.g. "re-enter editing there") should be short-circuited.
 #[derive(Debug)]
 pub struct AbortChain;
 impl Effect for AbortChain {
@@ -1170,10 +1099,8 @@ impl Effect for AbortChain {
 // ── Records hygiene ──────────────────────────────────────────────────────────
-/// Remove cells whose `CellKey` has no coordinates — these are meaningless
+/// Remove cells whose `CellKey` has no coordinates — only `AddRecordRow` with no
-/// records that can only be produced by `AddRecordRow` when no page
+/// page filters makes those. The inverse of the entry `SortData`.
 /// filters are set. Pushed by `ToggleRecordsMode` when leaving records
 /// mode, as the inverse of the `SortData` that runs on entry.
 #[derive(Debug)]
 pub struct CleanEmptyRecords;
 impl Effect for CleanEmptyRecords {
@@ -1276,11 +1203,8 @@ mod tests {
    // ── Storage injection (improvise-fuj) ───────────────────────────────
-    /// improvise-fuj: every effect that touches a document goes through
+    /// improvise-fuj: the whole document surface runs on a `Storage` that is not
-    /// `App::storage`, so the whole save/load/import/export surface works on
+    /// a filesystem. Fails if any of these effects reaches for `std::fs`.
    /// a backend that is not a filesystem. This is what a browser worker
    /// will supply (improvise-i34); here `MemoryFs` stands in for it, and
    /// the test would fail if any of these effects reached for `std::fs`.
    #[test]
    fn the_document_surface_runs_on_a_non_filesystem_backend() {
        use crate::storage::{MemoryFs, Storage};
@@ -1359,11 +1283,8 @@ mod tests {
        serde_json::to_string(&app.model_state.workbook.views).expect("views serialize")
    }
-    /// improvise-mae: a declared `kind()` is only worth having if it matches
+    /// improvise-mae: rather than restate each declaration, apply the effect and
-    /// what the effect actually does. Rather than restate each declaration,
+    /// check the shared state it claims not to touch is genuinely unchanged.
    /// this applies the effect and checks the shared state it claims not to
    /// touch is genuinely unchanged — a `ViewOnly` effect that quietly wrote
    /// a cell would never reach connected clients.
    #[test]
    fn view_only_effects_leave_shared_state_alone() {
        let cases: Vec<Box<dyn Effect>> = vec![
@@ -1408,9 +1329,8 @@ mod tests {
                model_before,
                "{effect:?} declares ViewOnly but changed model data"
            );
-            // No exemptions: since improvise-8uz moved the cursor out of
+            // No exemptions: since improvise-8uz moved the cursor out of `View`,
-            // View, SetSelected/SetRowOffset/SetColOffset touch nothing
+            // every effect in this list must clear the same bar.
            // shared, so every effect in this list must clear the same bar.
            assert_eq!(
                view_config_fingerprint(&app),
                views_before,
@@ -1419,15 +1339,8 @@ mod tests {
        }
    }
-    /// The structural effects, each pinned to the kind it must declare.
+    /// The structural effects, each pinned to the kind it must declare. The two
-    ///
+    /// "actually change" tests below cannot separate them — see the evidence.
    /// The bug this guards: `AddCategory` and `RemoveCategory` declared
    /// `CellData`, but both go through `Workbook`, whose `on_category_added`
    /// / `on_category_removed` rewrite **every** view's axis map. That is a
    /// re-layout, so a client told only "cell data changed" would keep
    /// rendering the old grid shape. The two "actually change" tests below
    /// cannot see it — a category change moves model data *and* view config,
    /// so it passes either list. Only pinning the declaration does.
    #[test]
    fn structural_effects_declare_their_kind() {
        let cell = ModelChange::CellData;
@@ -1582,12 +1495,8 @@ mod tests {
        assert!(items.contains(&"Electronics"));
    }
-    /// Bug (improvise-djn): `AddCategory` discarded the `Result` from
+    /// Bug (improvise-djn): `AddCategory` discarded the `Result`, so the panel
-    /// `Workbook::add_category`, so pressing `n` in the category panel with
+    /// reported its command's optimistic message and added nothing.
    /// the 12-regular-category limit already reached reported the optimistic
    /// `Added category "X"` its command had queued and added nothing. The
    /// effect runs last precisely so its failure can overwrite that message
    /// (improvise-6t3) — it just never wrote one.
    #[test]
    fn add_category_past_the_limit_reports_the_failure() {
        let mut app = test_app();
@@ -1651,9 +1560,8 @@ mod tests {
        assert!(!app.model_state.workbook.model.formulas().is_empty());
    }
-    /// Regression: AddFormula must add the target item to the target category
+    /// Regression: `AddFormula` must add the target item to the target category,
-    /// so the grid layout includes cells for it. Without this, a formula like
+    /// or the formula is registered but has no cells in the grid.
    /// `Margin = Profit / Revenue` would be registered but invisible in the grid.
    #[test]
    fn add_formula_adds_target_item_to_category() {
        let mut app = test_app();
@@ -1894,8 +1802,7 @@ mod tests {
        assert_eq!(app.view_state.mode, AppMode::Help);
    }
-    /// `AbortChain` must cause subsequent effects in the same
+    /// `AbortChain` skips the rest of the batch, and the flag must reset so the
    /// `apply_effects` batch to be skipped, and the flag must reset so the
    /// next dispatch starts clean.
    #[test]
    fn abort_chain_short_circuits_apply_effects() {
@@ -1920,10 +1827,8 @@ mod tests {
        assert_eq!(app.view_state.status_msg, "next-batch");
    }
-    /// BUG (improvise-8tk): `Save` reported failure only through the status
+    /// BUG (improvise-8tk): `Save` reported failure only to the status bar, so
-    /// bar, so the `Quit` that `wq`/`ZZ` chains after it landed anyway and
+    /// the chained `Quit` landed anyway and the session was discarded.
    /// the whole session was discarded without the user seeing the error.
    /// A failed save must abort the rest of the batch.
    #[test]
    fn save_without_file_path_aborts_the_chain() {
        let mut app = test_app();
@@ -1965,24 +1870,8 @@ mod tests {
        );
    }
-    /// BUG (improvise-lae, found reviewing it): improvise-8tk fixed **one
+    /// BUG (improvise-lae): improvise-8tk fixed **one arm** of a rule with two.
-    /// arm** of a rule with two. `Save` aborts the batch when the write
+    /// `:w /bad/path . q` builds exactly this batch — see the evidence.
    /// fails; `SaveAs` reported the error to the status bar and let the
    /// batch continue, which is the same silent discard 8tk closed — the
    /// shape `doctrine(one_arm_of_a_symmetric_rule_is_evidence_about_that_arm_only)`
    /// names, and the three tests above are evidence about `Save` only.
    ///
    /// It is reachable today: the `:` line composes with ` . `, so
    /// `:w /bad/path . q` builds exactly this batch, and
    /// `ExecuteCommand` concatenates the two commands' effects into one.
    /// improvise-lae's new save prompt also commits to `SaveAs`, so the
    /// arm was about to carry more traffic.
    ///
    /// Note what the abort does and does not buy the prompt: the typed path
    /// survives for the user to correct, but the message does not reach the
    /// screen, because `draw_bottom_bar` draws the minibuffer instead of the
    /// status line while a prompt is open. That is pre-existing and reported
    /// separately; the data-loss half is what this fixes.
    #[test]
    fn failed_save_as_aborts_the_chain() {
        let mut app = test_app();
@@ -2009,8 +1898,7 @@ mod tests {
        );
    }
-    /// The positive arm: a `SaveAs` that succeeds must not abort, or every
+    /// The positive arm: a `SaveAs` that succeeds must not abort. Without it,
    /// `:w <path>` would swallow whatever followed it. Without this,
    /// "always abort" passes the test above.
    #[test]
    fn successful_save_as_lets_the_chain_continue() {
@@ -2088,10 +1976,8 @@ mod tests {
        );
    }
-    /// `EnterEditAtCursor` must use its `target_mode` field, *not* whatever
+    /// `EnterEditAtCursor` uses its `target_mode` field, not whatever
-    /// `app.view_state.mode` happens to be when applied. Previous implementation
+    /// `app.view_state.mode` happens to be when it is applied.
    /// branched on `app.view_state.mode.is_records()` — the parameterized version
    /// trusts the caller (keymap or composing command).
    #[test]
    fn enter_edit_at_cursor_uses_target_mode_not_app_mode() {
        let mut app = test_app();
@@ -2123,12 +2009,8 @@ mod tests {
        );
    }
-    /// improvise-ehz: the edit buffer must be loaded from layout state that
+    /// improvise-ehz: the edit buffer must read layout that already reflects
-    /// already reflects earlier effects in the same batch. `apply_effects`
+    /// earlier effects in the batch, which is what a preceding `RebuildLayout` buys.
    /// only rebuilds *after* the batch, so a preceding `RebuildLayout` is
    /// what makes the cursor resolve to the post-mutation cell. Removing the
    /// row-0 item slides a different cell under the cursor; the buffer must
    /// show that cell's value, not the pre-batch one.
    #[test]
    fn enter_edit_at_cursor_reads_post_batch_layout() {
        let mut app = test_app();
@@ -2234,10 +2116,8 @@ mod tests {
        assert_eq!(app.view_state.buffers.get("edit").unwrap(), "hello");
    }
-    /// `SetBuffer` no longer treats "search" as an alias for the search query
+    /// `SetBuffer` does not treat "search" as an alias for the search query
-    /// (improvise-gdl): it is a plain buffer write whatever the name says, and
+    /// (improvise-gdl); the query is reached through `SetSearchQuery`.
    /// the query is reached through `SetSearchQuery`. Commands pick between
    /// the two in `cmd::core::write_buffer`.
    #[test]
    fn set_buffer_does_not_special_case_the_search_name() {
        let mut app = test_app();
@@ -2327,10 +2207,8 @@ mod tests {
        StartDrill(records).apply(&mut app);
        assert!(app.drill_state.is_some());
-        // Apply with no pending edits — should just clear state
+        // Through `apply_effects`, not `apply`: the dirty flag is raised off the
-        // Through `apply_effects`, not `apply`: the dirty flag is raised
+        // `EffectKind` (improvise-gv9r), so `apply` would assert nothing real.
        // off the `EffectKind` now (improvise-gv9r), so driving the effect
        // directly would assert a flag nothing in production sets.
        app.apply_effects(vec![Box::new(ApplyAndClearDrill)]);
        assert!(app.drill_state.is_none());
        assert!(
@@ -2373,9 +2251,8 @@ mod tests {
        }
        .apply(&mut app);
-        // Through `apply_effects`, not `apply`: the dirty flag is raised
+        // Through `apply_effects`, not `apply`: the dirty flag is raised off the
-        // off the `EffectKind` now (improvise-gv9r), so driving the effect
+        // `EffectKind` (improvise-gv9r), so `apply` would assert nothing real.
        // directly would assert a flag nothing in production sets.
        app.apply_effects(vec![Box::new(ApplyAndClearDrill)]);
        assert!(app.drill_state.is_none());
        assert!(app.model_state.dirty);
@@ -2385,10 +2262,8 @@ mod tests {
        );
    }
-    /// improvise-c5v: in wide format a records row groups one cell per
+    /// improvise-c5v: in wide format a records row groups one cell per measure,
-    /// measure, so renaming a coordinate has to move every one of them. The
+    /// so renaming a coordinate has to move every one of them.
    /// long-format code moved only the cell under the cursor, which would
    /// now split the row in half.
    #[test]
    fn apply_and_clear_drill_coord_rename_moves_every_measure_in_the_row() {
        let mut app = test_app();
@@ -2421,9 +2296,8 @@ mod tests {
        }
        .apply(&mut app);
-        // Through `apply_effects`, not `apply`: the dirty flag is raised
+        // Through `apply_effects`, not `apply`: the dirty flag is raised off the
-        // off the `EffectKind` now (improvise-gv9r), so driving the effect
+        // `EffectKind` (improvise-gv9r), so `apply` would assert nothing real.
        // directly would assert a flag nothing in production sets.
        app.apply_effects(vec![Box::new(ApplyAndClearDrill)]);
        assert!(app.model_state.dirty);
        let renamed = |measure: &str| {
@@ -2482,9 +2356,8 @@ mod tests {
        }
        .apply(&mut app);
-        // Through `apply_effects`, not `apply`: the dirty flag is raised
+        // Through `apply_effects`, not `apply`: the dirty flag is raised off the
-        // off the `EffectKind` now (improvise-gv9r), so driving the effect
+        // `EffectKind` (improvise-gv9r), so `apply` would assert nothing real.
        // directly would assert a flag nothing in production sets.
        app.apply_effects(vec![Box::new(ApplyAndClearDrill)]);
        assert_eq!(app.model_state.workbook.model.get_cell(&key), None);
    }
@@ -2652,10 +2525,8 @@ mod tests {
    // ── Headless import ─────────────────────────────────────────────────
-    /// BUG (improvise-k8i): ImportJsonHeadless parsed CSVs with `parse_csv`,
+    /// BUG (improvise-k8i): CSV short-row warnings went to stderr, invisible
-    /// which sends short-row warnings to stderr — invisible inside the TUI.
+    /// inside the TUI. They must surface in the status message.
    /// Short rows must surface in the status message, naming the affected
    /// column, never silently succeed.
    #[test]
    fn import_headless_csv_short_rows_surface_warning_in_status() {
        let dir = tempfile::tempdir().unwrap();
@@ -1,48 +1,5 @@
-//! The session loop: messages in, addressed messages out.
+//! The session loop: messages in, addressed messages out. One `App` per
-//!
+//! *document*, many clients per `App` — see `context/code/command.md`.
 //! A websocket is awkward to test, so nothing that decides anything lives in
 //! one. [`Host`] is a pure-ish state machine over the authoritative `App` —
 //! it takes one client's frame and returns the frames that owes every
 //! client — and [`crate::transport`] is the shell that moves bytes in and
 //! out of it. Every test in this crate drives `Host` directly; the socket is
 //! exercised by running the binary.
 //!
 //! **One `App` per document, many clients per `App`.** The ticket
 //! (improvise-q08) asked for one `App` per *connection*, "no shared
 //! authoritative state across sessions yet". That premise no longer holds
 //! (`doctrine(check_the_ticket_premise)`): `Sessions` exists precisely to
 //! hold many [`ClientSession`](crate::ui::project::ClientSession)s
 //! against one authoritative `App`, `apply_switch`/`apply_model` move every
 //! mirror in lockstep rather than the asker's
 //! (`doctrine(lockstep_binds_every_session_not_the_asker)`), and
 //! `Applied::Diff::broadcast` exists only because other clients must hear
 //! what this one did. An `App` per connection would make all of that dead
 //! code and give two browser tabs on the same document two documents.
 //!
 //! Nor is it one `App` per *server*. improvise is a multi-document app, and
 //! the unit of authority is the **document**: a `Host` is one `App`, one
 //! `Sessions`, and the clients attached to them. That grain is what
 //! `Sessions` was already built for — it never owns the `App`, every method
 //! takes it as a parameter — and it is what makes concurrency scope for
 //! free: sessions editing independent models cannot conflict, and the basis
 //! versions ADR 001 reasons about live on the `App`, one sequence per
 //! document (`document_generation` is the `App`'s).
 //!
 //! So a second document is a second `Host`, and hosting several means a
 //! registry of them keyed by document id. That registry is
 //! `improvise_ws_server::registry` (improvise-j31x), and it cost this file
 //! **nothing**:
 //! a `Host` was always a value rather than a singleton, and `handle` is
 //! already addressed to one document by virtue of being a method. What the
 //! registry adds above is the only thing a `Host` cannot supply — a
 //! server-wide address, since `SessionId`s are minted per `Sessions` and
 //! every document's first client is `SessionId(0)`.
 //!
 //! What sharing one document costs is honest and stated: there is no
 //! per-client permission, no conflict resolution beyond last-writer-wins,
 //! and a client that can connect can rewrite the document. ADR 001 owns
 //! where that goes next; this host is the localhost single-user case that
 //! ADR describes the starting point of.
 use crate::ui::app::App;
 use crate::ui::effect;
@@ -52,24 +9,16 @@ use improvise_protocol::{Command, Rejection};
 use improvise_protocol::{ClientFrame, ServerFrame};
-/// One outbound frame and the connection it goes to.
+/// One outbound frame and the connection it goes to. The host addresses by
-///
+/// [`SessionId`]; the transport owns the map from a session to a connection.
 /// The host addresses by [`SessionId`] and knows nothing about sockets; the
 /// transport owns the map from a session to a connection.
 #[derive(Debug, Clone, PartialEq)]
 pub struct Addressed {
    pub to: SessionId,
    pub msg: ServerFrame,
 }
-/// One authoritative document and every client attached to it.
+/// One authoritative document and every client attached to it — a value, never a
-///
+/// global. Not `Debug`: `App` is not, and a log line of the document is not wanted.
 /// The unit a multi-document server would key its registry by: a `Host` is
 /// a value, never a global, so hosting several is a map of these and not a
 /// change to anything inside one.
 ///
 /// Not `Debug`: `App` is not, and a `Host` printing the whole document into
 /// a log line would not be wanted even if it were.
 pub struct Host {
    app: App,
    sessions: Sessions,
@@ -87,17 +36,8 @@ impl Host {
        &self.app
    }
-    /// The authoritative `App`, for work the **host itself** initiates.
+    /// The authoritative `App`, for work the **host itself** initiates. Not a second
-    ///
+    /// door: a client's `Command` goes through [`Host::handle`] (gotchas 36, 43).
    /// Deliberately narrow in intent, and not a second door: a *client's*
    /// `Command` must go through [`Host::handle`], which routes it into
    /// `Sessions::apply` — the one place a basis is checked and the mirrors
    /// are kept in lockstep (gotchas 36, 43). What this is for is the work a
    /// host does on nobody's behalf, where there is no session to address and
    /// no claim to check: the browser worker writes the document back through
    /// the `Save` effect after a batch dirties it, because saving is
    /// `EffectKind::ViewOnly` and so reaches no wire vocabulary
    /// (`improvise-worker-server::worker`, which carries the argument).
    pub fn app_mut(&mut self) -> &mut App {
        &mut self.app
    }
@@ -106,13 +46,8 @@ impl Host {
        self.sessions.is_empty()
    }
-    /// Attach a client that has reported an extent of `rows` x `cols`, and
+    /// Attach a client that has reported an extent, and hand back its bootstrap.
-    /// hand back its bootstrap.
+    /// Total, and nothing is owed to the other clients.
    ///
    /// Total: there is no way for a connection to fail here, which is why
    /// the transport reads the extent off the first frame before calling.
    /// Nothing is owed to the *other* clients — a new connection changes no
    /// document state and moves no mirror but its own.
    pub fn connect(&mut self, rows: usize, cols: usize) -> (SessionId, Addressed) {
        let (id, snapshot) = self.sessions.connect(&self.app, rows, cols);
        (
@@ -124,11 +59,8 @@ impl Host {
        )
    }
-    /// A model command from a client that is fully caught up, for the tests
+    /// A model command from a client that is fully caught up, for the tests whose
-    /// whose subject is not ADR 001's basis. It goes through `handle` like
+    /// subject is not the basis. The staleness tests build their own `Basis`.
    /// any other frame; only the basis is supplied rather than folded, and
    /// [`caught_up`] can do that because it holds the `App` (a client
    /// cannot). The staleness tests build their own `Basis`.
    #[cfg(test)]
    fn model(&mut self, id: SessionId, cmd: improvise_protocol::ModelCommand) -> Vec<Addressed> {
        let wire = crate::ui::reduce::caught_up(&self.app, cmd);
@@ -145,43 +77,15 @@ impl Host {
        match msg {
            ClientFrame::Viewport { rows, cols } => self.resize(id, rows, cols),
            ClientFrame::Commands(cmds) => self.commands(id, &cmds),
-            // This arm answers nothing, and since improvise-bpq2 that is the
+            // Answering nothing is the settled answer, not a gap: host-side key
-            // settled answer rather than a gap. Resolving a keypress here
+            // resolution diverges (improvise-bpq2, gotcha 47).
            // needs `reduce_key` against the authoritative `App`, which emits
            // no `Command` a client could fold — so the two would diverge,
            // which `improvise-worker-server`'s
            // `a_key_resolved_on_the_host_moves_state_no_client_can_hear`
            // demonstrates. bpq2 closed that by moving resolution to the
            // client, so a key a client can answer never arrives as a `Key`
            // frame at all, and one that does is a binding nothing on this
            // side could answer without diverging. Growing the set is a
            // matter of enriching the projection until the client can
            // compose the command itself — see `improvise-wasm-client::keys`,
            // which carries the list of what is still upstream.
            ClientFrame::Key { .. } => Vec::new(),
            ClientFrame::Save => self.save(id),
        }
    }
-    /// Write the document back, because a client asked (improvise-368h).
+    /// Write the document back, because a client asked (improvise-368h). No session
-    ///
+    /// state moves and nobody else is told — see `context/code/command.md`.
    /// **No session state moves and nobody else is told**, which is why this
    /// does not go through `Sessions` at all: saving changes no document
    /// state and no client's view slice, so there is no mirror to adopt and
    /// nothing to broadcast. What moved is bytes in *this host's* storage —
    /// the same fact that makes `effect::Save` `EffectKind::ViewOnly` and the
    /// reason the `ModelCommand` derivation could never reach it (gotchas 3,
    /// 38). Success is silent, on the same terms as any accepted command that
    /// dirties no widget.
    ///
    /// Both refusals are the host's to answer, not `Checked`'s: neither is
    /// about a basis. The missing path is decided **before** anything runs,
    /// because a save that cannot land should be refused rather than
    /// attempted and then reported. The failed write can only be found by
    /// trying, and `Save` signals it by aborting its batch (gotcha 21) — the
    /// batch here is exactly `[Save]`, so an abort is that failure and
    /// nothing else. Reporting a failed write as success is the thing this
    /// exists to stop.
    fn save(&mut self, id: SessionId) -> Vec<Addressed> {
        let refused = match self.app.model_state.file_path.is_none() {
            true => Some(Rejection::NoDocumentPath),
@@ -201,37 +105,20 @@ impl Host {
            .collect()
    }
-    /// A resize dirties no document state at all — but it moves this
+    /// A resize dirties no document state, but it moves this client's viewport, and
-    /// client's viewport, and `ClientSession::project` emits the grid when
+    /// `project` emits the grid on that alone. Same fan-out, nothing dirty.
    /// the viewport moved even under `Dirty::NOTHING`. So the fan-out is the
    /// same call as everywhere else, with nothing dirty.
    fn resize(&mut self, id: SessionId, rows: usize, cols: usize) -> Vec<Addressed> {
        match self.sessions.get_mut(id) {
            Some(session) => session.set_extent(rows, cols),
-            // A frame for a session that has already gone. Nothing to do,
+            // A frame for a session that has already gone: there is no connection
-            // and nothing that could be done: there is no connection left to
+            // left to answer.
            // answer.
            None => return Vec::new(),
        }
        self.project(Dirty::NOTHING)
    }
-    /// A batch: **answered whole, then applied whole** (improvise-955n).
+    /// A batch: **answered whole, then applied whole** (improvise-955n). [`CheckedBatch`]
-    ///
+    /// carries what a basis means and the one residue this does not close.
    /// Order is why a batch exists at all: one keypress resolved on a client
    /// is `SetSelected` plus the scroll offsets that follow it, and a host
    /// that folded the selection but not the offset projects that client the
    /// wrong window.
    ///
    /// It used to apply in order and abandon at the first rejection, which
    /// left a hole ADR 001's "not even partially" does not have: a refused
    /// frame's *earlier* commands stayed applied. The check has moved from
    /// the command to the frame, so there is no longer a moment at which
    /// part of a frame has landed and the rest has been refused — the
    /// refusal, if any, happens before anything is applied, and
    /// `Sessions::apply_checked` cannot produce one.
    /// [`CheckedBatch`] carries what a batch's basis *means* and the one
    /// residue this does not close.
    fn commands(&mut self, id: SessionId, cmds: &[Command]) -> Vec<Addressed> {
        let batch = match CheckedBatch::against(&self.app, cmds) {
            Ok(batch) => batch,
@@ -251,14 +138,12 @@ impl Host {
        out
    }
-    /// The one door (gotcha 36), and the match that follows it is the
+    /// The one door (gotcha 36); the match that follows is the mechanism (gotcha 39):
-    /// mechanism (gotcha 39): asking for the `Dirty` means naming the case
+    /// asking for the `Dirty` means naming the case where a `Snapshot` is owed.
    /// where a `Snapshot` is owed instead.
    fn command(&mut self, id: SessionId, cmd: Checked<'_>) -> Vec<Addressed> {
        match self.sessions.apply_checked(&mut self.app, id, cmd) {
-            // The document was replaced. Nothing to diff against and nothing
+            // The document was replaced: nothing to diff and nothing to broadcast,
-            // to broadcast — each snapshot already carries a whole view slice
+            // each snapshot carrying a whole view slice and its seed.
            // and the seed that reproduces the new document.
            Applied::Rebased { snapshots } => snapshots
                .into_iter()
                .map(|(to, snapshot)| Addressed {
@@ -267,25 +152,14 @@ impl Host {
                })
                .collect(),
            Applied::Diff { dirty, broadcast } => {
-                // The broadcast precedes the projections, so a client's view
+                // The broadcast precedes the projections, so a client's slice is in
-                // slice is in the state the grid it is about to be sent was
+                // the state the grid it is about to get was built from (gotcha 36).
                // built from. Dropping it for the bystanders leaves their
                // slices naming the outgoing view while that grid comes from
                // the incoming one — a stale mirror arriving through
                // somebody else's keystroke (gotcha 36), which is the bug
                // PR #10 was found to have. Who each item reaches is carried
                // by the item, not decided here.
                let mut out = self.relay(id, broadcast);
                out.extend(self.project(dirty));
                out
            }
-            // Unreachable through this door since improvise-955n — a frame
+            // Unreachable through this door since improvise-955n, but a host that
-            // is refused before any of it is applied, by `commands` — but
+            // wants the `Dirty` must name every case (gotcha 39).
            // `Applied` is the shape `Sessions::apply` answers in and a host
            // that wants the `Dirty` must name every case (gotcha 39). The
            // answer is the same one `commands` gives: nothing was applied,
            // so nothing is owed to anybody else, and the asker is told only
            // that its batch did not land and which class that was.
            Applied::Rejected { reason } => vec![Addressed {
                to: id,
                msg: ServerFrame::Rejected(reason),
@@ -293,9 +167,8 @@ impl Host {
        }
    }
-    /// Who hears a relayed command is the relay's own business, not the
+    /// Who hears a relayed command is the relay's own business: [`Broadcast`] names
-    /// host's guess: [`Broadcast`] names its audience and the match is what
+    /// its audience and the match is what makes the host honour it (improvise-gvnb).
    /// makes the host honour it (improvise-gvnb).
    fn relay(&self, from: SessionId, broadcast: Vec<Broadcast>) -> Vec<Addressed> {
        broadcast
            .into_iter()
@@ -316,9 +189,8 @@ impl Host {
            .collect()
    }
-    /// Drain the outbound projections — the ticket's "projection queue",
+    /// Drain the outbound projections. Clients with nothing to hear are omitted by
-    /// which turned out to be a call rather than a queue. Clients with
+    /// `project_all` and so get no frame.
    /// nothing to hear are omitted by `project_all` and so get no frame.
    fn project(&mut self, dirty: Dirty) -> Vec<Addressed> {
        self.sessions
            .project_all(&self.app, dirty)
@@ -365,9 +237,8 @@ mod tests {
        Host::new(app)
    }
-    /// A host whose document can be replaced out from under its clients: a
+    /// A host whose document can be replaced out from under its clients: a second
-    /// second document sits in the storage backend under a name `LoadModel`
+    /// document in the storage backend under a name `LoadModel` can reach.
    /// can reach.
    fn host_over_a_loadable_document() -> (Host, PathBuf) {
        let storage = MemoryFs::new();
        let mut other = Workbook::new("Other");
@@ -389,9 +260,8 @@ mod tests {
        ])
    }
-    /// The same cell as a client names it: the opaque surrogate the server
+    /// The same cell as a client names it: the projected surrogate, which is the
-    /// projected on `CellDisplay::id`, which is the only handle a client has
+    /// only handle a client has (ADR 002). A `CellKey` is not constructible there.
    /// (ADR 002). A `CellKey` is not constructible over there.
    fn addressed(region: &str) -> RecordCell {
        RecordCell {
            record: RecordHash::of(&cell(region)),
@@ -399,22 +269,8 @@ mod tests {
        }
    }
-    /// The basis a caught-up client would state for a **frame** naming
+    /// The basis a caught-up client states for a **frame** naming `regions`. It
-    /// `regions`: the two document tiers plus each named record's current
+    /// fabricates `(hash, 0)` for a record that is absent — see the evidence.
    /// counter.
    ///
    /// `reduce::caught_up` cannot stand in for this, and the difference is
    /// exactly what a frame is: it builds the claim for **one command**, so a
    /// frame's records beyond the first are simply absent from it and come
    /// back `RecordUnstated`. A real client calls `BasisCache::claim` once
    /// with every record its frame names, which is what this reproduces.
    ///
    /// **Name only records that exist.** `claim` *omits* a record the cache
    /// has never heard of; this always includes one, so pointed at a record
    /// that is not there it fabricates `(hash, 0)` — a claim no real client
    /// could compose, and precisely the input that flips the create
    /// exemption in `CheckedBatch`. For a record the frame is *creating*,
    /// leave it out, which is what a client would do.
    fn frame_basis(host: &Host, regions: &[&str]) -> improvise_protocol::Basis {
        improvise_protocol::Basis {
            document_generation: host.app().document_generation(),
@@ -476,9 +332,8 @@ mod tests {
    // ── connect ─────────────────────────────────────────────────────────
-    /// AC 2: the bootstrap is sent on connect, addressed to the new client,
+    /// AC 2: the bootstrap is sent on connect, addressed to the new client, and it
-    /// and it is not empty — a `Snapshot` whose seed carried nothing would
+    /// is **not empty** — an empty seed would satisfy "a snapshot was sent".
    /// satisfy "a snapshot was sent" while leaving the client blank.
    #[test]
    fn connecting_hands_that_client_a_snapshot_that_draws_the_document() {
        let mut host = host();
@@ -497,9 +352,8 @@ mod tests {
        );
    }
-    /// The extent is what the snapshot is built at, so a client that says it
+    /// The extent is what the snapshot is built at, which is what makes `Viewport`
-    /// can see one row is not sent two. This is the property that makes
+    /// the handshake rather than an afterthought.
    /// `Viewport` the handshake rather than an afterthought.
    #[test]
    fn the_reported_extent_bounds_the_snapshot() {
        let mut host = host();
@@ -545,9 +399,8 @@ mod tests {
        assert_eq!(cell_text(&folded_grid(&seen), 0, 0), "42");
    }
-    /// `doctrine(check_the_checker)`: the assertion above would also pass if
+    /// `doctrine(check_the_checker)`: inject the fault — never send the command —
-    /// the seed happened to contain the value already. Inject the fault —
+    /// and the assertion above must fail.
    /// never send the command — and it must fail.
    #[test]
    fn without_the_command_the_cell_is_empty() {
        let mut host = host();
@@ -555,9 +408,8 @@ mod tests {
        assert_eq!(cell_text(&folded_grid(&[connect.msg]), 0, 0), "");
    }
-    /// A cell edit is the whole document's, so a client that did not ask for
+    /// A cell edit is the whole document's, so a client that did not ask is told
-    /// it is told about it too. The projection is per-session because the
+    /// too. The projection is per-session because viewports differ.
    /// viewports differ, not because the change does.
    #[test]
    fn a_bystander_is_projected_another_clients_edit() {
        let mut host = host();
@@ -577,9 +429,8 @@ mod tests {
        assert_eq!(cell_text(&folded_grid(&seen), 0, 0), "7");
    }
-    /// A view-only command reaches nobody else. The counterpart to the test
+    /// A view-only command reaches nobody else — the counterpart that keeps the
-    /// above: if every command fanned out, that one would pass for the wrong
+    /// test above from passing for the wrong reason.
    /// reason.
    #[test]
    fn a_scroll_reaches_only_the_client_that_scrolled() {
        let mut host = host();
@@ -598,15 +449,8 @@ mod tests {
        );
    }
-    /// One keypress resolved on a client is up to three commands, and the
+    /// One keypress is up to three commands and the host must take all of them
-    /// host must take all of them (improvise-bpq2).
+    /// (improvise-bpq2). The assertion is the *offset*, not the selection.
    ///
    /// The fixture is the shape that actually arrives: a cursor moved past
    /// the bottom of a one-row window, so the selection *and* the offset
    /// that follows it travel together. The assertion is the offset, not the
    /// selection, because a host that applied only the first command would
    /// still look right on `selected` — the window is what goes wrong, and
    /// it goes wrong silently.
    #[test]
    fn a_batch_of_commands_is_applied_whole_and_in_order() {
        let mut host = host();
@@ -629,10 +473,8 @@ mod tests {
        );
    }
-    /// A refused batch applies nothing *after* the refusal. The stronger
+    /// A refused batch applies nothing *after* the refusal. Nothing *before* it is
-    /// half — nothing *before* it either — is
+    /// the next test; the two halves fail independently.
    /// `a_refused_batch_leaves_the_commands_before_it_unapplied`; this is
    /// kept because the two halves fail independently.
    #[test]
    fn a_batch_is_abandoned_at_the_first_rejection() {
        use improvise_protocol::{Basis, ModelCommand};
@@ -664,29 +506,8 @@ mod tests {
        );
    }
-    /// improvise-955n / improvise-08wb. **The bug**: ADR 001 is emphatic
+    /// improvise-955n/08wb: `Checked` is per **command**, so a frame whose second
-    /// that a stale batch is never applied, *not even partially*, and
+    /// command was refused had already applied its first. See the evidence.
    /// improvise-ondj went to the trouble of a type (`Checked`) to make the
    /// unchecked path unconstructible. But `Checked` is **per command**, and
    /// a `ClientFrame::Commands` is a sequence of them: `Host::commands`
    /// applied in order and abandoned at the first rejection, so a frame
    /// whose *second* command was refused had already applied its first.
    /// The guarantee was true of a single command and false of a batch.
    ///
    /// The earlier command is a **view** command, which is not a
    /// concession: it is the shape the one real producer emits
    /// (`keys::commit` — one mutation among view commands), and it is still
    /// observable, because the host folds it into that client's mirror.
    /// A frame may carry more than one mutation; what it may not do is
    /// apply some of them and refuse the rest, which is clause 4 and open.
    ///
    /// **A refusal test needs an accepting one**
    /// (`doctrine(a_helper_that_builds_the_world_the_code_wants_tests_nothing)`:
    /// pair a refusal with an acceptance, or the refusal can pass by
    /// refusing everything). The second half runs the same leading
    /// `SetSelected` followed by a command whose basis is *current*, and
    /// requires both to land. Without it, a `SetSelected` that simply never
    /// worked would clear the first assertion and prove nothing at all.
    #[test]
    fn a_refused_batch_leaves_the_commands_before_it_unapplied() {
        use improvise_protocol::ModelCommand;
@@ -698,11 +519,8 @@ mod tests {
        let mut refused = host();
        let (asker, _) = refused.connect(10, 10);
-        // A claim that is current in every respect but one, so the refusal
+        // Built from `caught_up` and then bumped in a single tier, so the refusal
-        // can only be about the tier that was moved. Building it from
+        // can only be about the tier that moved.
        // `caught_up` and then bumping a single tier keeps the rest
        // well-formed — a hand-built basis that is stale in three ways at
        // once cannot tell which check answered.
        let Command::Model { mut basis, cmd } =
            crate::ui::reduce::caught_up(refused.app(), edit(42.0))
        else {
@@ -766,29 +584,8 @@ mod tests {
        );
    }
-    /// improvise-955n. **The regression 90ea26e introduced, and the reason
+    /// improvise-955n: hoisting the basis check to the frame hoisted *all* of it,
-    /// clause 2 cannot be applied to all three tiers.**
+    /// and the record tier does not hoist. A lost update — see the evidence.
    ///
    /// Hoisting the basis check to the frame hoisted *all* of it, so every
    /// mutation after the first was admitted with no record check at all.
    /// Two clients, one record: A composes `[SetCell(North), SetCell(South)]`
    /// against an honest current basis, B writes South, A's frame arrives —
    /// and A's write to South landed on top of B's **silently**, which is
    /// exactly the lost update ADR 001's record tier exists to prevent. At
    /// the base commit the second command was checked and the frame refused.
    ///
    /// **The per-command check was doing two jobs and the commit noticed
    /// one.** "Is the client's picture of the *document* current" is a
    /// frame-level fact and hoists correctly. "Is the client's picture of
    /// *this record* current" is not: which records are at stake is a
    /// property of the command, which is why `records_named` is an exhaustive
    /// match over `ModelCommand` (gotcha 43). The incapability argument that
    /// motivated clause 3 is true of records a frame **creates** and was
    /// over-generalised to records it merely **writes** — and those are the
    /// ones the tier is for.
    ///
    /// Three arms, because a fix that refuses everything would pass the first
    /// alone and a fix that checks nothing would pass the last two.
    #[test]
    fn a_later_write_in_a_chain_still_answers_for_its_own_record() {
        use improvise_protocol::ModelCommand;
@@ -818,10 +615,8 @@ mod tests {
            },
        ];
-        // B moves South. `SetCell` is `CellData`, so neither the document nor
+        // B moves South. `SetCell` is `CellData`, so only the record tier can
-        // the layout tier moves — only the record tier can discriminate here,
+        // discriminate here — which is what makes this a test of that tier.
        // which is what makes this a test of the record tier and not of the
        // hoist in general.
        moved.model(b, write("South", 99.0));
        let answer = moved.handle(a, ClientFrame::Commands(frame));
@@ -890,26 +685,7 @@ mod tests {
        );
        // ── arm 3: the same record twice in one chain ───────────────────
-        // Legitimate and likely — two measures on one record is one record —
+        // **Read the evidence before closing clause 4**: interleaving sells this.
        // and it works for a reason worth pinning rather than rediscovering:
        // **every check happens before any command is applied**, so the
        // second write is compared to the record's *pre-frame* counter, not
        // to the one its own sibling just bumped. Were the checks
        // interleaved with the applies, this frame would refuse itself with
        // `RecordMoved`.
        //
        // **Read this before closing clause 4.** Interleaving is tempting
        // there, because it would close the *silence*:
        // `[RemoveItem(North), SetCell(North)]` under interleaving is refused
        // `RecordGone` instead of silently half-applying, which is half of
        // what clause 4 owes. So check-before-apply is not a free win — it
        // buys the legitimate double-write below and **sells** the loud
        // failure on a broken chain, and clause 4 has to pay that back some
        // other way. Measured rather than argued: injecting the interleaving
        // fails exactly two tests, this one and
        // `a_frame_whose_later_mutation_fails_is_not_yet_undone`, and the
        // second failing *is* the trade. This arm is what stops someone
        // taking the trade without noticing they made it.
        let mut twice = host();
        let (asker, _) = twice.connect(10, 10);
        twice.model(asker, write("North", 1.0));
@@ -945,21 +721,8 @@ mod tests {
        );
    }
-    /// **The capability the contract is for**: a chain of data-dependent
+    /// **The capability the contract is for**: a data-dependent chain, in order.
-    /// commands, applied in order, each seeing what the previous one left.
+    /// The second assertion is the one that earns it — see the evidence.
    ///
    /// `[AddItem("East"), SetCell(the East record)]` is the archetype, and it
    /// is the case a per-command basis check **cannot** admit however it is
    /// ordered: a client cannot claim a counter for a record its own chain is
    /// about to create, because `BasisCache::claim` filters to records the
    /// cache has been told about. So the basis is answered once, on the first
    /// command that carries one, and the rest of the chain runs against what
    /// its predecessors left.
    ///
    /// The second assertion is the one that earns the test. Asserting only
    /// that the item exists would pass against a host that applied the first
    /// command and dropped the second — which is exactly what the previous
    /// rule did, loudly, and what the pre-frame check did, silently.
    #[test]
    fn a_data_dependent_chain_lands_in_order() {
        use improvise_protocol::ModelCommand;
@@ -1010,35 +773,8 @@ mod tests {
        );
    }
-    /// **Characterization of the open hole (clause 4), not an endorsement.**
+    /// **Characterization of the open hole (clause 4), not an endorsement.** Invert
-    ///
+    /// all three assertions when clause 4 lands — see the evidence.
    /// The contract says a frame is all-or-nothing. It is not yet:
    /// `App::apply_effects` aborts without undoing (gotcha 21 — an abort is
    /// not a rollback), so a chain whose later mutation fails leaves the
    /// earlier one standing. `[RemoveItem("North"), SetCell(the North
    /// record)]` is the shape: the removal lands, the write then resolves to
    /// nothing, `model_effects` returns an **empty** batch, and the frame is
    /// reported accepted having applied half of itself. Both the partial
    /// application and the silence are open.
    ///
    /// **This test did not pin that, and the way it failed is the repo's own
    /// signature defect sitting on the one test whose job is to fail loudly
    /// if anyone believes the hole is closed.** It asserted that North was
    /// removed and that nothing was rejected — both true of any *wholly
    /// successful* frame. The independent reviewer retargeted the second
    /// command from the removed North to the live South, making the frame
    /// succeed completely, and the test still passed. It could not tell
    /// half-applied from fully-applied, so it would never have inverted when
    /// clause 4 landed; it would have gone on passing. The missing assertion
    /// is the one that names the hole: **the second mutation did not land.**
    ///
    /// It is unreachable from any client here — the only producer of a
    /// multi-command frame is `keys::commit`, which emits one mutation among
    /// view commands — which is what makes it safe to document rather than
    /// rush.
    ///
    /// **This test must be inverted when clause 4 lands**: all three
    /// assertions flip, because then nothing at all happens.
    #[test]
    fn a_frame_whose_later_mutation_fails_is_not_yet_undone() {
        use improvise_protocol::ModelCommand;
@@ -1052,10 +788,8 @@ mod tests {
                value: CellValue::Number(1.0),
            },
        );
-        // The frame claims North at its current counter, so it passes the
+        // The frame claims North at its current counter, so it passes the check and
-        // check and fails at *apply* time — which is the only way to reach
+        // fails at *apply* time — the only way to reach the hole.
        // the hole. A frame refused by the check never applies anything and
        // would be testing clause 2 instead.
        let basis = frame_basis(&host, &["North"]);
        let answer = host.handle(
            asker,
@@ -1111,10 +845,8 @@ mod tests {
    // ── the broadcast ───────────────────────────────────────────────────
-    /// The bug Copilot found on PR #10: a bystander must fold the accepted
+    /// The bug Copilot found on PR #10: a bystander must fold the accepted command
-    /// command itself, because its own view slice lives on the client and
+    /// itself, because its own view slice lives on the client (gotcha 36).
    /// only `reduce_view` over the same command keeps it in step with the
    /// server's mirror (gotcha 36).
    #[test]
    fn a_bystander_is_sent_the_broadcast_and_the_asker_is_not() {
        let mut host = host();
@@ -1142,12 +874,8 @@ mod tests {
        );
    }
-    /// The other audience, and the reason [`Broadcast`] carries one rather
+    /// The other audience, and the reason [`Broadcast`] carries one: a geometry
-    /// than the relay deciding. A geometry change invalidates every cursor
+    /// change invalidates every cursor, the asker's included (improvise-gvnb).
    /// into the document (improvise-gvnb), and the asker's is not exempt: a
    /// `Command::Model` is the one kind a client cannot fold optimistically,
    /// so nothing has reset it locally and skipping it would leave the very
    /// client that asked positioned in a grid it no longer has.
    #[test]
    fn a_geometry_command_relays_the_cursor_reset_to_the_asker_too() {
        use improvise_protocol::ModelCommand;
@@ -1167,10 +895,8 @@ mod tests {
        }
    }
-    /// A refused batch is answered to the **asker and nobody else** (ADR
+    /// A refused batch is answered to the **asker and nobody else** (ADR 001):
-    /// 001). Nothing happened, so there is nothing to tell the others — and
+    /// nothing happened, so there is nothing to tell the others.
    /// the asker is told only which class of staleness it was, because it
    /// already holds what is current.
    #[test]
    fn a_stale_batch_is_refused_to_the_asker_alone() {
        use improvise_protocol::{Basis, Rejection};
@@ -1204,9 +930,8 @@ mod tests {
        );
    }
-    /// And what the broadcast is *for*: a bystander that folds what it is
+    /// And what the broadcast is *for*: a bystander that folds it names the view the
-    /// sent ends up naming the view the document accepted, so the cursor its
+    /// document accepted, so its next grid is positioned under the right key.
    /// next grid is positioned by is read under the right key (gotcha 24).
    #[test]
    fn folding_the_broadcast_moves_the_bystanders_active_view() {
        let mut host = host();
@@ -1236,9 +961,8 @@ mod tests {
    // ── the document swap ───────────────────────────────────────────────
-    /// AC of gotcha 39: a command that replaced the document owes *every*
+    /// AC of gotcha 39: a command that replaced the document owes *every* client a
-    /// client a fresh `Snapshot`, not a diff — including the client that did
+    /// fresh `Snapshot`, not a diff — including the client that did not ask.
    /// not ask.
    #[test]
    fn a_document_swap_sends_every_client_a_fresh_snapshot() {
        let (mut host, path) = host_over_a_loadable_document();
@@ -1261,9 +985,8 @@ mod tests {
        }
    }
-    /// `doctrine(check_the_checker)`: inject the fault the test targets — a
+    /// `doctrine(check_the_checker)`: inject the fault — a load that does not
-    /// load that does not happen — and the snapshot must not appear. A test
+    /// happen — and the snapshot must not appear.
    /// that saw `Snapshot` on every command would pass the one above.
    #[test]
    fn a_load_that_fails_is_not_a_rebase() {
        let (mut host, _) = host_over_a_loadable_document();
@@ -1308,18 +1031,8 @@ mod tests {
            .map(|bytes| String::from_utf8(bytes).expect("the fixture writes text"))
    }
-    /// improvise-368h. **The bug**: `effect::Save` is `EffectKind::ViewOnly`
+    /// improvise-368h: `effect::Save` is `ViewOnly`, so the `ModelCommand` derivation
-    /// — correctly, since it writes *this host's* path — so the derivation
+    /// could not reach it and no client could make its host write.
    /// that builds `ModelCommand` out of the `EffectKind::Model` effects
    /// could not reach it (gotcha 38). There was no save on the wire at all, so
    /// **no client on any transport could cause its host to write**: a
    /// websocket session edited the document in memory and the file on disk
    /// never moved unless a human was at a TUI that was not there.
    ///
    /// The derivation was not wrong; it was complete over the mutations and
    /// saving is not one. So the variant is *added* rather than derived, and
    /// it carries no path: a client cannot see the host's storage, and which
    /// document a host is hosting is the host's own fact.
    #[test]
    fn a_client_can_ask_its_host_to_save() {
        let (mut host, path) = host_with_a_path();
@@ -1351,14 +1064,8 @@ mod tests {
        );
    }
-    /// improvise-368h: the "what if there is no path" half. Not a panic and
+    /// improvise-368h, the "no path" half: refused with a reason, to the asker alone
-    /// not a silent no-op — the two answers the ticket ruled out — but the
+    /// (gotcha 39). The second client asserts the refusal does not leak.
    /// vocabulary that already exists for "nothing happened and here is
    /// why", addressed to the asker alone (gotcha 39).
    ///
    /// The second client is not decoration: a refusal that leaked to the
    /// bystanders would be telling them about a failure of somebody else's
    /// host-local request, and this asserts it does not.
    #[test]
    fn a_save_with_nowhere_to_write_is_refused_and_told_only_to_the_asker() {
        let mut host = host();
@@ -1381,14 +1088,8 @@ mod tests {
        );
    }
-    /// improvise-368h: a write that was attempted and failed must not be
+    /// improvise-368h: an attempted write that failed must not be reported as a
-    /// reported as a success. `Save` signals failure by aborting its batch
+    /// success. The fixture fails for a real reason — an absent directory.
    /// (gotcha 21), and the batch here is exactly `[Save]`, so an abort is
    /// that failure and nothing else.
    ///
    /// The fixture fails for a reason that is real rather than mocked: a
    /// `PhysicalFs` pointed at a directory that does not exist. It creates
    /// nothing, so the test leaves no trace on disk.
    #[test]
    fn a_write_that_fails_is_refused_rather_than_reported_as_saved() {
        let absent = std::env::temp_dir().join("improvise-no-such-directory-368h");
@@ -1440,9 +1141,8 @@ mod tests {
    // ── the frames themselves ───────────────────────────────────────────
-    /// The wire is JSON to start with (the ticket's choice, kept), so both
+    /// The wire is JSON, so both directions must survive a round trip through it —
-    /// directions must survive a round trip through it — including the one
+    /// including the one point at which `ViewState` is serialized (gotcha 25).
    /// point at which `ViewState` is serialized (gotcha 25).
    #[test]
    fn every_frame_survives_a_json_round_trip() {
        let mut host = host();
@@ -1468,11 +1168,8 @@ mod tests {
            assert_eq!(back, frame.msg);
        }
-        // Inbound, with a payload-bearing variant on each side of the
+        // A payload-bearing variant on each side of the envelope
-        // envelope (`doctrine(populate_before_roundtrip)`): a `Viewport` and
+        // (`doctrine(populate_before_roundtrip)`) — see the evidence.
        // a `Command` carrying a `CellValue::Text`, which is the arm the
        // outbound half cannot reach — a text cell aggregates to nothing
        // under a None-axis category, so it never becomes a `CellDisplay`.
        for inbound in [
            ClientFrame::Viewport { rows: 3, cols: 4 },
            ClientFrame::command(crate::ui::reduce::caught_up(
@@ -1,55 +1,5 @@
-//! The one place a command is applied to the authoritative state
+//! The one place a command is applied to the authoritative state, by
-//! (improvise-gxi).
+//! keypress or by wire (improvise-gxi). See `context/code/command.md`.
 //!
 //! Every deployment mode — the native TUI, a websocket server, a browser
 //! worker — reaches the model through the functions here, so there is one
 //! answer to "what does this input do" rather than one per host.
 //!
 //! One door, reached two ways, because input arrives in two shapes and only
 //! one of them is data on the wire:
 //!
 //! - [`reduce_full`] takes a [`Checked`], which is a [`Command`] — the wire
 //!   vocabulary — that the authoritative state has already answered for.
 //!   Both halves of that vocabulary are derived from the effects — the view
 //!   slice from the `ViewOnly` ones (`decision(improvise-cqi,
 //!   command_set_derived_not_invented)`) and the document from the
 //!   `EffectKind::Model` ones (improvise-kmgs) — so a command can now say
 //!   "set this cell" as well as "move my cursor".
 //! - [`reduce_key`] takes a keypress and runs it through the keymap,
 //!   registry and effect pipeline. A keypress names an *intent* whose
 //!   meaning depends on the `CmdContext` (where the cursor is, what mode is
 //!   current); a `Command` names the mutation that intent resolved to. That
 //!   is why the key path survives the vocabulary rather than being replaced
 //!   by it.
 //!
 //! What is no longer two is the *mutation*: [`model_effects`] maps a
 //! [`ModelCommand`] back to the effect it was derived from, and both paths
 //! run it through `App::apply_effects`. Effects remain the only place the
 //! `App` is written.
 //!
 //! Both return the commands to send onward: the projections for the widgets
 //! whose content is the document's rather than one client's. The grid is
 //! per-viewport and comes from [`Sessions::project_all`](super::project::Sessions::project_all)
 //! instead — a host with connected clients uses the [`Dirty`] the inner
 //! functions return and projects per session; a host with none, like the
 //! native TUI, drops what it gets.
 //!
 //! **A wire command arrives here through [`Checked`], and nowhere else**
 //! (improvise-ondj, ADR 001). The basis check is a property of the session
 //! door rather than of the mutation path — a host holding the `App` *is* the
 //! truth and not a claimant — but "the mutation path does not check" was
 //! implemented as a second door rather than as an exemption: both functions
 //! used to take a `&Command` and their `Command::Model { cmd, .. }` arm
 //! discarded the basis with `..`, which is the syntax for ignoring a field
 //! and costs no compile error. So the answer is not a guard but a type: the
 //! only way to obtain a `Checked` over a `Command::Model` is
 //! [`Checked::against`], which runs [`stale_basis`] against the
 //! authoritative `App`. A host that wants to apply a client's batch must
 //! face the [`Rejection`], and `Sessions::apply_checked` is the door a host
 //! goes through — `Host::commands` answers the whole frame first with
 //! [`CheckedBatch::against`], so `Sessions::apply` (which answers one command
 //! and then dispatches) now has no production caller and is kept for the
 //! single-command case its tests drive.
 use improvise_core::model::record::RecordHash;
 use improvise_protocol::{Basis, Command, ModelCommand, Rejection, reduce_view};
@@ -59,202 +9,20 @@ use crate::ui::app::App;
 use crate::ui::effect::{self, Effect};
 use crate::ui::project::{Dirty, document_projections};
-/// A wire [`Command`] the authoritative state has answered for.
+/// A wire [`Command`] whose basis the authoritative state has answered for
-///
+/// (improvise-ondj/7v62, ADR 001). See `context/code/command.md`.
 /// The one thing it makes unrepresentable: a [`Command::Model`] applied
 /// without its basis being checked (improvise-ondj). The inner field is
 /// private and [`Checked::against`] is the only constructor, so there is no
 /// spelling of "apply this batch and ignore what it claims" — the `..` that
 /// used to discard the basis is now a `Rejection` the caller must handle.
 ///
 /// It is **not** a claim about which `App` was asked: a host with two
 /// documents could check against one and apply to the other. That is not a
 /// hole this closes, because a `Sessions` is already bound to one document's
 /// generation sequence (ADR 001) and a `Registry` hands out one pair at a
 /// time.
 ///
 /// # Why the lifetime does not close it (improvise-7v62)
 ///
 /// The obvious answer — make the lifetime the `&App`'s, so the borrow
 /// checker forbids checking one document and applying to another — was
 /// tried, and it is not merely inconvenient: it is **contradictory with what
 /// the type is for**. A `Checked` is a proof discharged *before* a
 /// mutation, and its whole job is to be carried across that mutation; a
 /// borrow of the `App` is a claim that must stay live *during* it. The two
 /// cannot both hold. Tying them makes every call site an `E0502`, starting
 /// with the canonical one:
 ///
 /// ```text
 /// error[E0502]: cannot borrow `*app` as mutable because it is also borrowed as immutable
 ///     match Checked::against(app, cmd) {
 ///                            --- immutable borrow occurs here
 ///         Ok(checked) => self.apply_checked(app, id, checked),
 ///                        ^^^^^ mutable borrow occurs here
 /// ```
 ///
 /// A generative brand would express it without borrowing, but that needs a
 /// closure-scoped API and would distort every host. The remaining option is
 /// carrying a document identity, and `DocumentId` lives in the *server's*
 /// registry: an `App` does not have one and does not want one, precisely
 /// because the unit of authority is one document per `App`. So this is a
 /// **deliberate won't-fix**, not an oversight. What keeps it unreachable is
 /// stated above and is load-bearing — improvise-j31x made this a
 /// multi-document server on purpose, and ADR 001 never evicts a document so
 /// that a reconnecting client cannot have a stale basis accepted as current.
 pub struct Checked<'a>(&'a Command);
-/// A whole [`ClientFrame::Commands`](improvise_protocol::ClientFrame) the
+/// A whole frame, answered before any of it is applied (improvise-955n/08wb,
-/// authoritative state has answered for — **every** command in it, before
+/// ADR 001). Its four clauses are in `context/code/command.md`.
 /// any of them is applied (improvise-955n/improvise-08wb, ADR 001).
 ///
 /// [`Checked`] alone was not enough, and the gap was exactly one level up
 /// from the one improvise-ondj closed. A `Checked` answers for *one*
 /// command against the `App` **as it stands**; a frame is a sequence of
 /// them, applied in order, so a frame whose second command was refused had
 /// already applied its first. ADR 001 says a stale batch is never applied,
 /// *not even partially*, and that was true of a single command and false of
 /// a batch.
 ///
 /// # One basis, checked once, and then the chain runs
 ///
 /// The contract, as the user stated it: *"a sequence of commands sent to the
 /// server should assume that it's played in order from start to end and that
 /// each command sees the state after the previous command runs"*, sent as
 /// *"a single transaction from a given base"*. Four clauses:
 ///
 /// 1. A frame states **one basis**, describing the base the client composed
 ///    against.
 /// 2. That basis is checked **once**, against that base, before anything is
 ///    applied.
 /// 3. Commands are applied **in order**, and command N sees the state command
 ///    N-1 left. Data dependence between them is expected, not an error.
 /// 4. **All or nothing.** If any command fails, the frame leaves the document
 ///    exactly as the base described.
 ///
 /// Clauses 1–3 are here. **Clause 4 is not yet implemented** — see the hole
 /// below and the design note on PR #26.
 ///
 /// # Two of the three tiers hoist, and the third does not
 ///
 /// Clause 2 hoists the check, but **not every tier is a frame-level fact**,
 /// and a first version of this hoisted all three. That was a lost update:
 /// `document_generation` and `layout_version` describe the *document*, so
 /// answering them once for the frame is exactly right — but the record tier
 /// asks "is the client's picture of **this record** current", and which
 /// records are at stake is a property of the *command*. That is why
 /// [`records_named`] is an exhaustive match over [`ModelCommand`] and not
 /// something a frame could summarise (gotcha 43).
 ///
 /// Skipping it for later commands let `[SetCell(North), SetCell(South)]`
 /// overwrite a South that somebody else had moved, silently, with the frame
 /// reported accepted — the precise loss ADR 001's record tier exists to
 /// prevent, and invisible to the other two tiers because a `SetCell` is
 /// `CellData` and moves neither of them. So **every** mutation answers for
 /// its own records, against the **frame's** basis: the one the first
 /// basis-carrying command stated, not each command's own field, so a later
 /// command cannot state a fresher claim than the frame was composed under.
 ///
 /// ## Except the records the frame is creating
 ///
 /// The argument that motivated clause 3 is true, and it is true of a
 /// narrower thing than it was first applied to. A client cannot claim a
 /// counter for a record its own chain is about to **create**, because
 /// [`BasisCache::claim`](improvise_protocol::BasisCache::claim) filters to
 /// records the cache has been told about — so `[AddItem(East),
 /// SetCell(East)]` would be refused by a naive per-command check however it
 /// is ordered. But that argument was then stretched to records the frame
 /// merely **writes**, and those are the ones the tier is for.
 ///
 /// So the exemption is stated over the *property* rather than over a list of
 /// commands: a record that **neither resolves in the document nor is claimed
 /// by the basis** is one the frame is about to create, and is not answered
 /// for. Every other combination still is — including a record that does not
 /// resolve but *is* claimed, which is a client believing in something that
 /// has been removed, and is `RecordGone` as before. See
 /// [`will_be_created_by_the_frame`].
 ///
 /// What the base means is therefore: **what the client saw**. The transaction
 /// promises the whole chain lands on top of it or none of it does. The
 /// chain's internal coherence is the transaction's job, not the basis
 /// check's.
 ///
 /// # The hole clause 4 leaves, stated rather than hidden
 ///
 /// `App::apply_effects` aborts without undoing (gotcha 21 — an abort is not a
 /// rollback), so a frame whose second mutation fails leaves the first
 /// applied. `[RemoveItem("North"), SetCell(the North record)]` is the
 /// example: the removal lands, the write then resolves to nothing and
 /// `model_effects` returns an **empty** batch, so the frame is reported
 /// accepted having applied half of itself. Both halves of that are open —
 /// the partial application and the silence.
 ///
 /// The create exemption has a residue of the same kind and it is the same
 /// hole: a later command naming a record that is neither present nor claimed
 /// is *presumed* to be one the frame creates, so if the frame does not in
 /// fact create it, the write resolves to nothing and silently does not
 /// happen. That is `model_effects`' empty batch again, not a second defect,
 /// and it closes when clause 4 makes a mid-chain failure reportable.
 ///
 /// Both are **unreachable from any client in this workspace**: the only
 /// producer of a multi-command frame is
 /// `improvise-wasm-client::keys::commit`, which emits one mutation among view
 /// commands. **Reachability is a reason not to rush a fix and not a reason
 /// not to notice one** — the lost update above had the same reachability
 /// argument as this hole and none of its documentation, which is how it
 /// shipped. That is what makes it safe to
 /// document rather than rush, and it is why an earlier "at most one document
 /// mutation per frame" rule — which closed the hole by forbidding the
 /// capability — was rejected.
 ///
 /// # Two things the rule that was rejected got wrong, worth keeping
 ///
 /// **Arity is not a property of a variant.** A future `Command::RunScript` is
 /// one variant performing an unbounded number of mutations, so no count over
 /// the vocabulary can be right: a classifier can honestly answer *whether* a
 /// command changes the document, never *how many times*. A
 /// `mutates_the_document` predicate written for the rejected rule is deleted
 /// with it rather than kept unused — but it was made an **exhaustive match**
 /// on review, because a `matches!` answers `false` for a future variant in
 /// silence, and clause 4 will need that classifier back. Write it exhaustive
 /// when it returns.
 ///
 /// **The transaction boundary is not the frame.** Because one command may
 /// already be a chain, clause 4 is a property the system needs *inside* a
 /// command as much as across a frame of them, which is why the design note
 /// puts the mechanism at `App::apply_effects` rather than here.
 pub struct CheckedBatch<'a>(Vec<Checked<'a>>);
 impl<'a> CheckedBatch<'a> {
-    /// Answer for every command in a frame, against the state the frame was
+    /// The document tiers are answered once, on the first command that
-    /// composed against — which is the state *before* any of them runs.
+    /// carries a basis; the record tier is asked of every mutation.
    ///
    /// The refusal is the frame's answer and nothing is applied. It is a
    /// `Result` rather than a per-command verdict for the same reason
    /// [`Checked`] is a type rather than a guard: the only enforceable
    /// version of "answer before applying" is not being able to reach the
    /// commands without having done so.
    ///
    /// **The two document tiers are answered once**, on the first command
    /// that carries a basis — clause 2 — and every later command is admitted
    /// without re-asking them, which is clause 3. **The record tier is asked
    /// of every mutation**, against that same frame basis, because it is not
    /// a frame-level fact. A frame with no mutation answers trivially.
    ///
    /// The `Basis` on a *later* `Command::Model` is ignored rather than
    /// compared, and that is an honest interim rather than the end state:
    /// clause 1 says the **frame** states one basis, but the wire still
    /// carries a `Basis` inside each `Command::Model` — right when one
    /// command was one transaction, and now one field per command describing
    /// something that belongs to the frame. Moving it to
    /// `ClientFrame::Commands` would make "one basis per frame"
    /// unrepresentable-otherwise instead of a convention, and that protocol
    /// change belongs with clause 4.
    pub fn against(app: &App, cmds: &'a [Command]) -> Result<Self, Rejection> {
-        // The frame's basis: the one its first mutation stated. Later
+        // The frame's basis is the one its first mutation stated; a later
-        // commands carry a `Basis` field too, and it is deliberately *not*
+        // command's own `Basis` field is deliberately not read (clause 1).
        // read — clause 1 says the frame states one base, and reading each
        // command's own would let a chain claim a fresher picture than it was
        // composed under.
        let mut frame_basis: Option<&Basis> = None;
        let mut checked = Vec::with_capacity(cmds.len());
        for cmd in cmds {
@@ -283,21 +51,8 @@ impl<'a> IntoIterator for CheckedBatch<'a> {
 }
 impl<'a> Checked<'a> {
-    /// Answer for a command, against the state it would be applied to.
+    /// Answer for a command, against the state it would be applied to. Total
-    ///
+    /// over the vocabulary; only [`Command::Model`] can be refused.
    /// Total over the vocabulary, and only [`Command::Model`] can be
    /// refused: everything else is either this client's own view slice or a
    /// `SwitchView`, which carries no basis and cannot corrupt — the
    /// workbook simply refuses a view it does not have (ADR 001 names that
    /// second hole and leaves it open deliberately).
    ///
    /// Saving is not in this vocabulary and deliberately so: it is
    /// `ClientFrame::Save`, answered by the host, because it mutates no
    /// shared state and has no basis to answer for (improvise-368h).
    ///
    /// There is no argument that suppresses the answer. ADR 001's force is a
    /// fresh batch under a fresh basis, composed by the client; it is not a
    /// way past this constructor.
    pub fn against(app: &App, cmd: &'a Command) -> Result<Self, Rejection> {
        let refused = match cmd {
            Command::Model { basis, cmd: model } => stale_basis(app, basis, model),
@@ -309,19 +64,8 @@ impl<'a> Checked<'a> {
        }
    }
-    /// A command in a frame whose **document** tiers have already been
+    /// A command in a frame whose document tiers are already answered
-    /// answered (clause 2), answering only for its own records.
+    /// (clause 2), answering only for its own records. Private, not a bypass.
    ///
    /// The two document tiers are not re-asked, which is clause 3: they
    /// describe a document this command is not running against, since its
    /// predecessors have moved it. The record tier *is* asked, because it is
    /// a fact about the command rather than the frame — see
    /// [`CheckedBatch`], where the lost update that taught this is recorded.
    ///
    /// Private, and that is what keeps it from being a bypass: from outside
    /// this module the only doors are still [`Checked::against`] and
    /// [`CheckedBatch::against`], and the only caller is the loop that has
    /// just answered the frame's basis.
    fn within_a_frame(app: &App, basis: &Basis, cmd: &'a Command) -> Result<Self, Rejection> {
        let refused = match cmd {
            Command::Model { cmd: model, .. } => stale_records_in_a_frame(app, basis, model),
@@ -333,52 +77,24 @@ impl<'a> Checked<'a> {
        }
    }
-    /// The command this proof is about.
+    /// The command this proof is about, borrowed from `'a` rather than from
-    ///
+    /// `&self` so a caller can dispatch and still move the proof onward.
    /// It hands back a `&'a Command` rather than one borrowed from `&self`,
    /// so a caller can dispatch on the variant and still move the proof into
    /// the function it dispatched to. Crate-private: a proof that could be
    /// unwrapped from outside would be a proof nobody had to obtain.
    pub(crate) fn command(&self) -> &'a Command {
        self.0
    }
 }
-/// Apply one checked wire command to the authoritative state, and say what
+/// Apply one checked wire command to the authoritative state, and say what it
-/// it dirtied.
+/// dirtied. An inbound `Command::Project` is ignored: projections travel one way.
 ///
 /// `Command::SwitchView` does not go through `reduce_view` alone: the
 /// document's "which view is active" and the client's must move together,
 /// and `App::switch_to_view` is one of the only two writers allowed to touch
 /// either (gotcha 29). It reaches that writer the way every other mutation
 /// here does — through its effect — so the view history a switch records is
 /// the same on both doors. Everything else is pure view slice.
 ///
 /// An inbound `Command::Project` is ignored. Projections travel
 /// server→client; a client that sends one is describing state it does not
 /// own.
 pub fn apply_command(app: &mut App, cmd: Checked<'_>) -> Dirty {
    match cmd.0 {
        Command::Project(_) => Dirty::NOTHING,
        Command::SwitchView(name) => {
-            // The mutation is `effect::SwitchView`'s, not a second copy of
+            // The mutation is `effect::SwitchView`'s, not a second copy: the
-            // it: the effect records the view history the key path leaves
+            // effect records the history `ModelCommand::ViewBack` pops.
            // behind, and `ModelCommand::ViewBack` pops exactly that stack.
            // Switching by calling `App::switch_to_view` here instead left
            // the stack empty, so a wire client could not go back where a
            // keyboard one could — see
            // `a_wire_view_switch_records_history_the_way_a_key_switch_does`.
            // `apply_effects` rebuilds the layout for us.
            app.apply_effects(vec![Box::new(effect::SwitchView(name.clone()))]);
-            // The `Dirty` is deliberately narrower than the
+            // Deliberately narrower than `Dirty::of(Model(LayoutGeometry))`:
-            // `Dirty::of(Model(LayoutGeometry))` that call returns. That
+            // a switch leaves the view list alone (improvise-pca).
            // table is a per-*kind* over-approximation — sound for the kind
            // because `CreateView` and `DeleteView` wear it too — and a view
            // switch is the one member that leaves the view list alone: its
            // names and axes belong to the document, and which view is
            // active is not in `ViewListCache` at all (improvise-pca). Over-
            // projecting costs bandwidth, never staleness, so narrowing here
            // is safe; widening `Dirty::of` for this one member would not be.
            Dirty {
                grid: true,
                category_tree: true,
@@ -386,8 +102,8 @@ pub fn apply_command(app: &mut App, cmd: Checked<'_>) -> Dirty {
                ..Dirty::NOTHING
            }
        }
-        // The basis was answered for by `Checked::against`; there is no
+        // The basis was answered for by `Checked::against`; there is no other
-        // other way to have got here with this variant.
+        // way to have got here with this variant.
        Command::Model { cmd, basis: _ } => {
            let effects = model_effects(app, cmd);
            app.apply_effects(effects)
@@ -399,29 +115,8 @@ pub fn apply_command(app: &mut App, cmd: Checked<'_>) -> Dirty {
    }
 }
-/// The other half of the derivation: a [`ModelCommand`] back to the effect
+/// The other half of the derivation: a [`ModelCommand`] back to the effect it
-/// it was derived from (improvise-kmgs).
+/// was derived from (improvise-kmgs). Exhaustive on purpose.
 ///
 /// This is the seam that keeps a command from becoming a second way to
 /// mutate. A `ModelCommand` names a mutation; it does not perform one. The
 /// effect it names is the same value the keymap path builds, and it is run
 /// through the same [`App::apply_effects`], so both doors go through the
 /// one mutation site and both get their [`Dirty`] from the effect's own
 /// `EffectKind` rather than from a second classification that could drift.
 ///
 /// The match is exhaustive on purpose: a new `ModelCommand` variant fails
 /// the build here until it names an effect, exactly as a new effect fails
 /// the build until it classifies itself (gotcha 3).
 ///
 /// It takes the `App` because two variants name their operand by
 /// **surrogate** rather than by coordinates (ADR 002), and resolving one is
 /// a lookup against the authoritative state. Resolution failing is not an
 /// error path bolted on: it is one of the two arms of staleness — the record
 /// is gone, a category or item it named was removed — and the answer is an
 /// empty batch, which applies nothing. `Sessions::apply` now reaches that
 /// same conclusion first and typed, as `Rejection::RecordGone` — this stays
 /// as the floor under the key path and under any host that bypasses the
 /// session door.
 pub fn model_effects(app: &App, cmd: &ModelCommand) -> Vec<Box<dyn Effect>> {
    let one: Box<dyn Effect> = match cmd {
        ModelCommand::SetCell { cell, value } => {
@@ -515,26 +210,8 @@ pub fn model_effects(app: &App, cmd: &ModelCommand) -> Vec<Box<dyn Effect>> {
    vec![one]
 }
-/// Which records a [`ModelCommand`] writes, decided by the **server**
+/// Which records a [`ModelCommand`] writes, decided by the **server**, never
-/// (ADR 001, improvise-fkfg).
+/// read off the client's claim (ADR 001, improvise-fkfg). Exhaustive on purpose.
 ///
 /// This is deliberately not "whichever records the client's basis mentions".
 /// A client that simply left a record out of its claim would otherwise have
 /// a **force by omission** — the record tier skipped, with no flag to
 /// remove and nothing in the message saying so — and force is supposed to be
 /// a re-composition against the current basis, never a bypass of the check.
 /// The server decides what is checked; the client only supplies what it saw.
 ///
 /// The match is exhaustive, and the long list of "names no record" is the
 /// point rather than noise: a new variant that writes a record fails the
 /// build here until it says so. That matters because the failure mode is
 /// silent — an unlisted record-writing variant would simply never be checked
 /// against its own tier, and the suite would stay green.
 ///
 /// Only `CellData` variants that carry a [`RecordCell`] name a record. The
 /// item and formula commands are `CellData` too but name no *record*: they
 /// move the axes a record's coordinates are drawn from, which is what the
 /// layout tier is for.
 pub fn records_named(cmd: &ModelCommand) -> Vec<RecordHash> {
    match cmd {
        ModelCommand::SetCell { cell, .. } | ModelCommand::ClearCell(cell) => vec![cell.record],
@@ -566,24 +243,8 @@ pub fn records_named(cmd: &ModelCommand) -> Vec<RecordHash> {
    }
 }
-/// Is this batch's basis still current? `None` means apply it.
+/// Is this batch's basis still current? `None` means apply it. The three tiers
-///
+/// are checked largest-first, so the answer names the largest thing that moved.
 /// The three tiers in order of how much they invalidate, so the answer a
 /// client is given names the largest thing that moved rather than an
 /// arbitrary one: a document swap is not usefully reported as "a record
 /// moved".
 ///
 /// The record tier separates **structural** from **data** staleness, and
 /// keeping them apart is the whole reason ADR 002 put a maintained
 /// `RecordHash` index in `DataStore` rather than regenerating one from the
 /// layout. A record that does not resolve is *gone* — there is nothing to
 /// write to and nothing coherent to force, because force never creates
 /// structure. A record that resolves at a different version merely *changed*,
 /// and force there is one keystroke.
 ///
 /// Nothing here mutates, and there is no argument that makes it return
 /// `None` regardless: a stale batch is never applied, and ADR 001's force is
 /// a fresh batch under a fresh basis rather than a way past this function.
 pub fn stale_basis(app: &App, basis: &Basis, cmd: &ModelCommand) -> Option<Rejection> {
    let document_moved = (basis.document_generation != app.document_generation())
        .then_some(Rejection::DocumentReplaced);
@@ -597,11 +258,7 @@ pub fn stale_basis(app: &App, basis: &Basis, cmd: &ModelCommand) -> Option<Rejec
 }
 /// The record tier for a command inside a frame whose document tiers are
-/// already answered.
+/// already answered, skipping the records the frame is about to create.
 ///
 /// It reuses [`stale_record`] unchanged — one home for what each of the four
 /// combinations means — and differs only in *which* records it asks about:
 /// the ones the frame is about to create are skipped.
 fn stale_records_in_a_frame(app: &App, basis: &Basis, cmd: &ModelCommand) -> Option<Rejection> {
    records_named(cmd)
        .into_iter()
@@ -609,20 +266,8 @@ fn stale_records_in_a_frame(app: &App, basis: &Basis, cmd: &ModelCommand) -> Opt
        .find_map(|hash| stale_record(app, basis, hash))
 }
-/// Is this a record the frame is about to create?
+/// Is this a record the frame is about to create? **A property, not a list of
-///
+/// commands** — see `context/code/command.md`. Only the pair exempts.
 /// **A property, not a list of commands.** The document does not have it and
 /// the basis claims nothing about it — which is precisely what "I am creating
 /// this" looks like from here, because `BasisCache::claim` can only state a
 /// counter for a record the client has been told about.
 ///
 /// The near miss is the other half of that conjunction: a record that does
 /// not resolve but *is* claimed is a client believing in something that has
 /// been removed, and stays [`Rejection::RecordGone`]. Only the pair exempts.
 ///
 /// It is asked **only inside a frame**, never of a lone command: a first
 /// command has no predecessor that could have created anything, so
 /// [`Checked::against`] still refuses a write to a record that is not there.
 fn will_be_created_by_the_frame(app: &App, basis: &Basis, hash: RecordHash) -> bool {
    app.resolve_record(hash).is_none() && basis.record(hash).is_none()
 }
@@ -637,20 +282,8 @@ fn stale_record(app: &App, basis: &Basis, hash: RecordHash) -> Option<Rejection>
    }
 }
-/// The command a client that is **fully caught up** would compose.
+/// The command a client that is **fully caught up** would compose. It takes
-///
+/// `&App`, so nothing reachable from a wire can call it: not a bypass.
 /// It reads the tiers straight off the `App`, which looks like forging and
 /// is not: it takes `&App`, so only the side that *is* the authoritative
 /// state can call it, and for that side "the current basis" is not a claim
 /// to be checked but a fact. A remote client has no `App` and no other
 /// constructor — `BasisCache::claim` withholds until the server has spoken —
 /// so nothing reachable from a wire can get here.
 ///
 /// It exists for two callers: a host that resolves a command locally and
 /// wants to push it through the same door a client's would take, and the
 /// tests whose subject is not staleness. The tests whose subject *is*
 /// staleness build the [`Basis`] by hand, so neither kind can pass by
 /// accident through the other's convenience.
 pub fn caught_up(app: &App, cmd: ModelCommand) -> Command {
    Command::Model {
        basis: Basis {
@@ -665,12 +298,8 @@ pub fn caught_up(app: &App, cmd: ModelCommand) -> Command {
    }
 }
-/// Apply one checked wire command and return what every client must be told.
+/// Apply one checked wire command and return the document-scoped projections.
-///
+/// Per-client grid updates need a viewport and come from `Sessions::project_all`.
 /// The signature the three hosts share. The returned commands are the
 /// document-scoped projections; per-client grid updates need a session's
 /// viewport and come from `Sessions::project_all`, driven by the [`Dirty`]
 /// that [`apply_command`] returns.
 pub fn reduce_full(app: &mut App, cmd: Checked<'_>) -> Vec<Command> {
    let dirty = apply_command(app, cmd);
    document_projections(app, dirty)
@@ -706,11 +335,8 @@ mod tests {
        App::new(wb, None)
    }
-    /// For the tests whose subject is not staleness: a command the current
+    /// A command the current state accepts. The staleness tests build their
-    /// state accepts, which every command built by [`caught_up`] is by
+    /// own [`Rejection`] instead, so neither kind borrows the other's luck.
    /// construction. The tests whose subject *is* staleness call
    /// [`Checked::against`] themselves and assert on the [`Rejection`], so
    /// neither kind can pass by accident through the other's convenience.
    fn checked<'a>(app: &App, cmd: &'a Command) -> Checked<'a> {
        Checked::against(app, cmd).expect("composed against the current state")
    }
@@ -745,9 +371,7 @@ mod tests {
    }
    /// The two copies of "which view is active" must move together
-    /// (gotcha 29). Folding `SwitchView` through `reduce_view` alone would
+    /// (gotcha 29), or the cursor map is read under the wrong key.
    /// move the client's and leave the document's behind, and the cursor map
    /// would then be read under the wrong key.
    #[test]
    fn switch_view_moves_the_document_and_the_client_together() {
        let mut app = app();
@@ -756,23 +380,8 @@ mod tests {
        assert_eq!(app.model_state.workbook.active_view, "Second");
    }
-    /// A wire-driven view switch owes the same history a key-driven one
+    /// A wire-driven view switch owes the same `view_back_stack` history a
-    /// leaves behind.
+    /// key-driven one leaves behind (Copilot, PR #11).
    ///
    /// `ModelCommand::ViewBack` pops `view_state.view_back_stack`, and the
    /// only thing that ever pushes onto it is `effect::SwitchView`. While
    /// `apply_command` performed the switch itself — calling
    /// `App::switch_to_view` directly rather than running the effect — the
    /// stack stayed empty, so a client that switched over the wire could not
    /// go back, though the same two keypresses could. Wave 4 is what made
    /// that reachable: before `ModelCommand` there was no wire way to ask.
    ///
    /// Nothing else in the suite could see it.
    /// `switch_view_moves_the_document_and_the_client_together` checks where
    /// a switch *landed*; this checks what it *recorded*.
    ///
    /// Bug demonstrated 2026-09-19 (Copilot, PR #11): the `ViewBack` left the
    /// document on "Second".
    #[test]
    fn a_wire_view_switch_records_history_the_way_a_key_switch_does() {
        let mut app = app();
@@ -908,12 +517,8 @@ mod tests {
        }
    }
-    /// `app()` with a cell actually stored, so a surrogate for it resolves
+    /// `app()` with a cell actually stored, so a `RecordCell` surrogate for it
-    /// through the index rather than only through the layout. The walks over
+    /// resolves — otherwise every walk below fails for a reason not its own.
    /// `every_model_command` run against this: a `SetCell` whose record
    /// cannot be resolved produces *no effects*, and every one of those
    /// tests asserts on the effects, so the fixture has to name something
    /// real or they would all fail for a reason that is not theirs.
    fn model_app() -> App {
        let mut app = app();
        app.model_state
@@ -924,12 +529,8 @@ mod tests {
        app
    }
-    /// One value per `ModelCommand` variant.
+    /// One value per `ModelCommand` variant. [`variant_name`] matches
-    ///
+    /// exhaustively, so a new variant fails the build until it is listed here.
    /// The enum is the derived set, so a test that walks "the set" has to
    /// walk all of it. [`variant_name`] is the completeness guard: it matches
    /// exhaustively, so a variant added to the protocol fails the build here
    /// until it is listed below as well.
    fn every_model_command() -> Vec<ModelCommand> {
        vec![
            ModelCommand::SetCell {
@@ -1031,15 +632,8 @@ mod tests {
        }
    }
-    /// The derivation, stated as a check: every `ModelCommand` names an
+    /// The derivation, stated as a check: every `ModelCommand` names an effect
-    /// effect that declares `EffectKind::Model(..)`.
+    /// that declares `EffectKind::Model(..)` — not a `ViewOnly` one (gotcha 35).
    ///
    /// This is what "derived, not invented" means operationally
    /// (`doctrine(derive_the_enum_from_the_effects)`). A variant pointed at a
    /// `ViewOnly` effect would be a wire message that changed the document
    /// for the client that sent it and for nobody else — `Dirty::of` would
    /// report `NOTHING`, and no projection would ever correct the others
    /// (gotcha 35).
    #[test]
    fn every_model_command_names_an_effect_that_declares_a_model_change() {
        for cmd in every_model_command() {
@@ -1060,26 +654,12 @@ mod tests {
        }
    }
-    /// The two `Model` effects whose operand is **session-local residue**
+    /// The two `Model` effects whose operand is session-local residue
-    /// (gotcha 30): one client's uncommitted drill staging, and the workbook
+    /// (gotcha 30): one client's drill staging, one client's wizard workbook.
    /// one client's half-finished import wizard built.
    const SESSION_LOCAL_EFFECTS: [&str; 2] = ["ApplyAndClearDrill", "AcceptWizardModel"];
-    /// improvise-3x3u: the wire vocabulary is the *addressable* model
+    /// improvise-3x3u: the wire vocabulary is the *addressable* model effects.
-    /// effects, not all of them.
+    /// Re-adding either variant means addressing it, not restoring the nullary form.
    ///
    /// `Sessions::apply` routes a `Command::Model` with no `SessionId`,
    /// which is right exactly when the change is addressed to no one session
    /// (gotcha 36). These two are addressed to precisely one, and it is not
    /// the sender: an unaddressed `ApplyAndClearDrill` commits another
    /// client's private typing (improvise-tom1) and an unaddressed
    /// `AcceptWizardModel` swaps the document out of a dialogue the sender
    /// never opened. Both were demonstrated against `Sessions::apply` before
    /// the variants were removed.
    ///
    /// Re-adding either variant means making it *addressed* -- carrying a
    /// `SessionId`, with the staging map moved onto the session -- not
    /// restoring the nullary form this test forbids.
    #[test]
    fn no_model_command_commits_another_clients_session_local_state() {
        for cmd in every_model_command() {
@@ -1097,8 +677,7 @@ mod tests {
    }
    /// The guard above is only worth having while the effects it names still
-    /// exist and are still model changes -- otherwise it would pass by
+    /// exist and are still model changes (`doctrine(check_the_checker)`).
    /// naming nothing (`doctrine(check_the_checker)`).
    #[test]
    fn the_session_local_effects_are_real_model_effects() {
        let effects: [Box<dyn Effect>; 2] = [
@@ -1118,14 +697,8 @@ mod tests {
        }
    }
-    /// The type name of an effect, read off the leading identifier of its
+    /// An effect's type name, off the leading identifier of its derived `Debug`.
-    /// derived `Debug`.
+    /// `Effect` is a trait object, so `std::any::type_name` cannot reach it.
    ///
    /// `Effect` is a trait object, so `std::any::type_name` is not reachable
    /// through it; every effect derives `Debug`, and a derived `Debug` opens
    /// with the type's own name for a unit struct, a tuple struct and a
    /// braced struct alike. So the name is already on the value — this only
    /// reads it back.
    fn effect_type_name(effect: &dyn Effect) -> String {
        format!("{effect:?}")
            .split(|c: char| !c.is_alphanumeric() && c != '_')
@@ -1134,10 +707,8 @@ mod tests {
            .to_string()
    }
-    /// The reader above is itself a check, so it gets the injection too
+    /// The reader above is itself a check, so it gets the injection too: one
-    /// (`doctrine(check_the_checker)`): one effect of each `Debug` shape,
+    /// effect of each `Debug` shape — nothing, `(`, and ` {` after the name.
    /// because the leading identifier is followed by a different delimiter
    /// in each — nothing, `(`, and ` {`.
    #[test]
    fn the_effect_name_reader_reads_all_three_debug_shapes() {
        assert_eq!(effect_type_name(&effect::TransposeAxes), "TransposeAxes");
@@ -1154,29 +725,8 @@ mod tests {
        );
    }
-    /// The mapping is name-for-name by construction, so state that: every
+    /// Every `ModelCommand` variant names the effect of the same name. This is
-    /// `ModelCommand` variant names the effect that carries its own name.
+    /// the check that separates an effect from its inverse — see the evidence.
    ///
    /// This is the check that separates an effect from its inverse.
    /// [`every_model_command_names_an_effect_that_declares_a_model_change`]
    /// cannot: `HideItem` and `ShowItem`, `AddItem` and `RemoveItem`,
    /// `CreateView` and `DeleteView`, `SetCell` and `ClearCell` are each two
    /// effects of the *same* `EffectKind`, so a swapped pair clears that bar
    /// while inverting what the wire message does. And
    /// [`a_model_command_lands_where_its_keypress_lands`] cannot reach them
    /// either — most variants have no single keypress to compare against, so
    /// it drives three of twenty-eight.
    ///
    /// Proven by injection 2026-09-19: `HideItem` → `effect::ShowItem`,
    /// `CreateView` → `effect::DeleteView` and `AddItem` →
    /// `effect::RemoveItem` all left `cargo test --workspace` at 918 passing,
    /// 0 failing. This test fails on each.
    ///
    /// The name-for-name rule is a design constraint, not a coincidence, and
    /// it is what makes the derivation checkable at all
    /// (`doctrine(derive_the_enum_from_the_effects)`): a variant whose effect
    /// must be named something else is a variant that stopped being derived,
    /// and it should fail here and be argued about rather than pass quietly.
    #[test]
    fn every_model_command_names_the_effect_of_the_same_name() {
        for cmd in every_model_command() {
@@ -1202,22 +752,12 @@ mod tests {
        assert_eq!(names.len(), total, "a variant is listed twice: {names:?}");
    }
-    /// The point of the whole ticket: a model `Command` applied
+    /// A model `Command` applied authoritatively lands where the keypress bound
-    /// authoritatively leaves the document exactly where the keypress bound
+    /// to the same mutation lands, `Dirty` included.
    /// to the same mutation leaves it. That equivalence is what lets the two
    /// entry points share one door.
    ///
    /// The `Dirty` is compared too, because a host with connected clients
    /// projects off it: a command that produced the right document and the
    /// wrong `Dirty` would leave every other client stale.
    #[test]
    fn a_model_command_lands_where_its_keypress_lands() {
-        /// Two categories on different axes, because the one-category
+        /// Two categories on different axes: a one-category fixture cannot tell
-        /// fixture cannot tell a transpose from a cycle: with only `Region`
+        /// a transpose from a cycle (`doctrine(check_the_checker)`).
        /// on Row, swapping the axes and rotating Row→Column land on the
        /// same view, and an equivalence test that cannot distinguish the
        /// wrong effect from the right one proves nothing
        /// (`doctrine(check_the_checker)`).
        fn pivoted() -> App {
            let mut app = app();
            app.model_state.workbook.add_category("Month").unwrap();
@@ -1279,23 +819,8 @@ mod tests {
        );
    }
-    /// A wire `Command::Model` cannot reach the document without its basis
+    /// A wire `Command::Model` cannot reach the document unchecked (ADR 001,
-    /// being checked (ADR 001, improvise-ondj).
+    /// improvise-ondj). The regression guard is the signature, not this test.
    ///
    /// Bug demonstrated 2026-09-19: `reduce_full` and `apply_command` — the
    /// functions this module's own doc points every future host at, the
    /// browser worker included — took a `&Command` and matched
    /// `Command::Model { cmd, .. }`, so a batch composed against a document
    /// that had since moved landed anyway. The `..` discarded the basis with
    /// no compile error.
    ///
    /// **The regression guard is the signature, not this test.** Both now
    /// take a [`Checked`], whose only constructor runs [`stale_basis`], so
    /// the version of this test that reproduced the bug —
    /// `reduce_full(&mut app, &wire)` with a stale `wire` — no longer
    /// compiles. What is left to assert is that the constructor refuses, and
    /// that refusing leaves the document alone: there is no half-applied
    /// batch, because the type is obtained before anything is applied.
    #[test]
    fn a_stale_wire_model_command_cannot_be_applied() {
        let mut app = app();
@@ -1333,33 +858,8 @@ mod tests {
        );
    }
-    /// improvise-gv9r. **The bug**: a client's edit left the authoritative
+    /// improvise-gv9r: a client's edit used to leave the document reporting
-    /// document reporting *clean*, on every transport.
+    /// *clean* on every transport. All three `ModelChange` levels, separately.
    ///
    /// `MarkDirty` used to be a separate effect that the *command* layer
    /// composed alongside the mutation, and improvise-kmgs derived
    /// `ModelCommand` from the effects **one at a time** — so the wire got
    /// `SetCell` without its companion. A derivation correct per element and
    /// wrong for a set. "Save if dirty" was therefore a trigger that never
    /// fired, and a host built on it loses every client edit silently; the
    /// browser worker found that out the hard way and had to over-approximate
    /// off the command variant instead.
    ///
    /// The fix is the unrepresentable one: dirtiness is raised by
    /// `App::apply_effects` off the same `EffectKind` the `Dirty` and the two
    /// basis tiers are read from (gotcha 40), so no path can compose a
    /// document change without it. There is nothing left to forget, and
    /// `MarkDirty` is gone rather than left as a second answer.
    ///
    /// The three levels are asserted separately because they are three
    /// different effects: an effect that is only ever reached by one of them
    /// would otherwise pass on a sibling's evidence. The negative arm — that
    /// a `ViewOnly` effect leaves the document clean — is
    /// `a_view_only_batch_leaves_the_document_clean` in `ui/app.rs`, and it
    /// has to live there: a view-slice `Command` never reaches
    /// `apply_effects` at all (`apply_command` folds it straight into
    /// `ViewState`), so a negative written against this door would pass
    /// however unconditional the raise was.
    #[test]
    fn a_wire_model_command_marks_the_document_dirty() {
        for cmd in [
@@ -1387,16 +887,8 @@ mod tests {
        }
    }
-    /// `SwitchView` is the one command with a foot in both halves, and it is
+    /// `SwitchView` persists as the `Initial View` line, so it dirties the
-    /// the reason this matters beyond cell edits: `Workbook::active_view`
+    /// document too — and the worker's persist trigger depends on that.
    /// persists as the `Initial View` line, so a switch really does put the
    /// document out of step with the file.
    ///
    /// It is tested on its own because the browser worker's persist trigger
    /// now depends on it (improvise-gv9r): the worker used to name
    /// `SwitchView` explicitly in a table of variants and now asks the
    /// document. If this effect were ever reclassified `ViewOnly`, the
    /// worker would silently stop writing view switches back.
    #[test]
    fn a_wire_view_switch_marks_the_document_dirty() {
        let mut app = app();
@@ -13,15 +13,12 @@ use crate::view::{AxisEntry, GridLayout};
 pub const MIN_COL_WIDTH: u16 = 5;
 pub const MAX_COL_WIDTH: u16 = 32;
-// The row-header bounds moved to `improvise-protocol` with the rule that
+// The row-header bounds live in `improvise-protocol` with the rule that clamps to
-// clamps to them (improvise-1125): a browser sizing its own row header must
+// them (improvise-1125): a browser must reach the same numbers.
 // reach the same numbers, and it cannot depend on this crate.
 pub use improvise_protocol::{MAX_ROW_HEADER_W, MIN_ROW_HEADER_W};
-/// Compute adaptive column widths for pivot mode (header labels + cell values).
+/// Adaptive column widths for pivot mode. Header widths use the widest *individual*
-/// Header widths use the widest *individual* level label (not the joined
+/// level label, not the joined multi-level string.
 /// multi-level string), matching how the grid renderer draws each level on
 /// its own row with repeat-suppression.
 pub fn compute_col_widths(
    model: &Model,
    layout: &GridLayout,
@@ -80,15 +77,8 @@ pub fn compute_col_widths(
        .collect()
 }
-/// The total row-header width for a layout.
+/// The total row-header width for a layout. The rule is the protocol's; this is the
-///
+/// keypress-path caller that borrows tuples — see `context/code/command.md`.
 /// The rule itself is `improvise_protocol::header_level_widths`, which is
 /// where both front ends reach it (improvise-1125). This is the one caller
 /// that holds a `GridLayout` rather than a projected frame —
 /// `App::visible_cols`, deciding how many columns fit — and it is on the
 /// keypress path, so it hands the rule **borrowed** tuples rather than
 /// building a frame and cloning every row. It does not reach for a `View`
 /// either, because a collapsed flag cannot change a width.
 pub fn compute_row_header_width(layout: &GridLayout) -> u16 {
    let items = layout.row_items.iter().filter_map(|e| match e {
        AxisEntry::DataItem(v) => Some(v),