test(protocol): make the cyclic all() prove it reaches every variant (improvise-5ubs)

AppMode::all() and Rejection::all() walk a total cyclic after(), and both doc
comments claimed a new variant could not be omitted. Half true, wrong half:
answering after() FOR a new variant is compiler-forced, pointing an existing
arm AT it is not, so a variant nothing names is never walked -- measured by
the PR #78 reviewer as all().len() == 17 against 18 variants, suite green.

Closing it needs an enumeration independent of after(). Stable Rust has none
for an enum with data -- variant_count is nightly, a derive macro is a new
proc-macro crate, and strum's EnumIter cannot construct AppMode::ItemAdd or
Editing at all. serde can: the Deserialize derive already builds the complete
variant-name list from the enum definition and publishes it in its
"unknown variant ... expected one of ..." message. variants.rs reads that back
and diffs it against the names the cycle reached, each taken off the variant's
own serialization rather than off Debug.

The helper is defensive because an unparsed message would return an empty list,
and an empty list passes every cycle; a should_panic test points it at a struct
and asserts the message, not merely that it panics.

Both overclaiming doc comments replaced. The fourth instance of this shape --
assert_eq!(cases.len(), 13) in ui/effect.rs -- is over a hand-built list of
effects rather than enum variants, so serde has nothing to say about it; left
filed rather than bodged.

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

context/code/protocol.md

@@ -98,6 +98,50 @@ types into its minibuffer.
 `every_hint_key_resolves_in_its_mode` in `improvise-command`; `Quit` alone
 shows nothing, because it is never rendered.
 
+### `all()` walks a cycle, and its totality is tested rather than structural
+
+`AppMode::all()` and `Rejection::all()` walk a total, cyclic `after()`. Both
+doc comments used to claim a new variant could not be omitted, because
+`after()` has no `None` arm. **That is half true and the wrong half.**
+Answering `after()` *for* a new variant is compiler-forced; pointing an
+existing arm *at* it is not, so a variant nothing names is simply never
+walked — `all().len()` came back 17 where the enum had 18, with the suite
+green (improvise-5ubs). The general statement is
+`doctrine(a_list_checked_for_length_reads_like_a_list_checked_for_completeness)`:
+an `assert_eq!(cases.len(), 13)` catches a case being *deleted* from a list
+and never one being *absent* from it, and the two are written identically.
+
+Closing it needs an enumeration independent of `after()`, and without one the
+ticket's own first option restates the problem. There is no such enumeration
+in stable Rust for an enum with data — `variant_count` is nightly, a derive
+macro means a new proc-macro crate, and `strum`'s `EnumIter` cannot construct
+these variants at all, since `AppMode::ItemAdd` carries a category name and
+`Editing` a whole `MinibufferConfig`. What there *is* is **serde**: the
+`Deserialize` derive already builds the complete variant-name list, from the
+enum definition, maintained by the compiler, and publishes it in its
+`unknown variant … expected one of …` message. `variants.rs` reads it back
+and diffs it against the names the cycle actually reached.
+
+Three things this costs, and they are the reason the module is defensive
+rather than three lines. The message is **parsed**, so a serde whose wording
+changes must fail loudly instead of reporting an empty list that passes every
+cycle — hence the `unknown variant` prefix assertion, the `names.len() > 1`
+floor and `a_message_that_is_not_an_unknown_variant_is_refused`, which points
+it at a struct. A variant is identified by **its own serialization** rather
+than by `Debug`, because an externally tagged unit variant is a string and a
+struct variant a one-key object, and reading a name off `Debug` would depend
+on the field list. And it checks only that every declared variant is
+*reached*: an `after()` that is not injective — two arms naming one successor
+— is a different defect this does not see, and would show up as a shorter
+cycle only if it also orphaned something.
+
+What it does **not** reach: the fourth instance of this shape,
+`layout_geometry_effects_actually_change_document_config` in
+`improvise-command/src/ui/effect.rs`, whose `assert_eq!(cases.len(), 13)` is
+over a hand-built list of *effects*, not enum variants, so serde has nothing
+to say about it. Filed rather than bodged, because a fourth hand-rolled
+attempt in a fourth place is the disease.
+
 ---
 
 ## 2. `reduce.rs` — the wire vocabulary

context/test-evidence/protocol.md

@@ -462,3 +462,39 @@ against an enum that really did have the variant.
 
 Proved discriminating by re-adding the variant with an empty `apply` arm: the
 retired spelling parses again and the test fails on `expect_err`.
+
+
+### boundary_tests::the_app_mode_cycle_reaches_every_variant_serde_declares
+
+### basis::tests::the_rejection_cycle_reaches_every_variant_serde_declares
+
+improvise-5ubs. Both `all()` functions walk a cyclic `after()`, and both doc
+comments claimed a new variant could not be omitted. Answering `after()` for a
+new variant is compiler-forced; pointing an existing arm at it is not, so a
+variant nothing names is never walked. The reviewer of PR #78 measured it:
+`AppMode::all().len()` came back 17 against 18 variants, suite green.
+
+The independent enumeration is serde's — the `Deserialize` derive builds the
+variant-name list from the enum definition and publishes it in its
+`unknown variant ... expected one of ...` message. `variants.rs` reads it back
+and diffs it against the names the cycle reached, each name taken off the
+variant's own serialization rather than off `Debug`.
+
+Fault injected 2026-09-21: a `Rejection::Probe` variant added to the enum, with
+an `after()` arm returning `DocumentReplaced` and **nothing pointing at it** —
+precisely the shape the reviewer probed. `the_rejection_cycle_reaches_every_variant_serde_declares`
+fails naming `["Probe"]`; the other 120 tests in `improvise-protocol`'s lib pass,
+including `the_rejection_cycle` half of every basis test. (The workspace does not
+compile with the probe, because `improvise-wasm-client/src/socket.rs` matches
+`Rejection` exhaustively — which is a *different* guard and does not catch the
+omission, since the omitted variant there is one it was forced to name.)
+
+### variants::tests::a_message_that_is_not_an_unknown_variant_is_refused
+
+The checker's own checker. A parse of serde's message that silently fails would
+return an empty name list, and an empty list is a subset of everything — every
+cycle would pass and the guard would be worse than absent. Pointing `declared`
+at `Basis`, a struct, produces a message with no `expected one of` clause; the
+helper must panic rather than return. Asserting the panic *message* and not
+merely that it panics, because a helper that panicked for some other reason
+would satisfy a bare `should_panic`.

crates/improvise-protocol/src/basis.rs

@@ -107,7 +107,7 @@ impl Rejection {
     }
 
     /// Every variant, one sample each, walked rather than listed — the same
-    /// shape `AppMode::all` uses, and for the same reason.
+    /// shape `AppMode::all` uses, with the same tested-not-structural totality.
     pub fn all(sample: RecordHash) -> Vec<Self> {
         let mut out = vec![Self::DocumentReplaced];
         let mut next = Self::DocumentReplaced.after(sample);
@@ -118,8 +118,8 @@ impl Rejection {
         out
     }
 
-    /// The next variant in the cycle [`Rejection::all`] walks. Total: with no
-    /// `None` arm, answering for a new variant means naming another one.
+    /// The next variant in the cycle [`Rejection::all`] walks. A new variant must
+    /// answer here; that it is also POINTED AT is what §1's test checks.
     fn after(&self, sample: RecordHash) -> Self {
         match self {
             Self::DocumentReplaced => Self::LayoutMoved,
@@ -215,4 +215,11 @@ mod tests {
         );
         assert_eq!(one, other);
     }
+
+    /// improvise-5ubs, the Rejection half: the same cyclic omission, the same
+    /// independent enumeration. Evidence: `context/test-evidence/protocol.md`.
+    #[test]
+    fn the_rejection_cycle_reaches_every_variant_serde_declares() {
+        crate::variants::cycle_is_total(&Rejection::all(hash("East")), "Rejection");
+    }
 }

crates/improvise-protocol/src/lib.rs

@@ -13,6 +13,8 @@ pub mod keymap;
 mod reduce;
 pub mod render;
 pub mod resolve;
+#[cfg(test)]
+pub(crate) mod variants;
 pub mod viewmodel;
 
 pub use basis::{Basis, BasisCache, Rejection};
@@ -237,8 +239,8 @@ impl AppMode {
         }
     }
 
-    /// Every variant, one sample each. Walked rather than listed, so a new
-    /// variant cannot be added without answering `after` — §1 of the doc.
+    /// Every variant, one sample each, walked rather than listed. Totality is
+    /// **tested, not structural** — `context/code/protocol.md` §1, improvise-5ubs.
     pub fn all() -> Vec<Self> {
         let mut out = vec![Self::Normal];
         let mut next = Self::Normal.after();
@@ -258,8 +260,8 @@ impl AppMode {
             .collect()
     }
 
-    /// The next variant in the cycle [`AppMode::all`] walks. Total on purpose —
-    /// `context/code/protocol.md` says what that does and does not buy.
+    /// The next variant in the cycle [`AppMode::all`] walks. A new variant must
+    /// answer here; that it is also POINTED AT is what §1's test checks.
     fn after(&self) -> Self {
         match self {
             Self::Normal => Self::editing(),
@@ -521,4 +523,11 @@ mod boundary_tests {
         portable::<super::KeyCode>();
         portable::<super::KeyModifiers>();
     }
+
+    /// improvise-5ubs: a cyclic `after()` can silently omit a variant nothing
+    /// points at. Evidence: `context/test-evidence/protocol.md`.
+    #[test]
+    fn the_app_mode_cycle_reaches_every_variant_serde_declares() {
+        crate::variants::cycle_is_total(&super::AppMode::all(), "AppMode");
+    }
 }

crates/improvise-protocol/src/variants.rs

@@ -0,0 +1,88 @@
+//! The one enumeration of an enum's variants that the **compiler** maintains:
+//! serde's derive. Why this and not a list — `context/code/protocol.md` §1.
+
+use serde::Serialize;
+use serde::de::DeserializeOwned;
+
+/// A name no enum here has, so deserializing it is always the error arm.
+const ABSENT: &str = "\"__no_such_variant__\"";
+
+/// Every variant name `T`'s `Deserialize` derive knows, read off its
+/// `unknown variant` message. Panics rather than returning an empty list.
+pub fn declared<T: DeserializeOwned>() -> Vec<String> {
+    let message = serde_json::from_str::<T>(ABSENT)
+        .err()
+        .expect("a name no variant has must fail to deserialize")
+        .to_string();
+    let (head, tail) = message
+        .split_once(", expected one of ")
+        .unwrap_or_else(|| panic!("serde no longer says `unknown variant`: {message}"));
+    assert!(
+        head.starts_with("unknown variant"),
+        "serde no longer says `unknown variant`: {message}"
+    );
+    let names: Vec<String> = tail
+        .split(", ")
+        .map(|name| name.trim_start_matches('`').to_string())
+        .map(|name| match name.split_once('`') {
+            Some((name, _)) => name.to_string(),
+            None => name,
+        })
+        .collect();
+    assert!(
+        names.len() > 1,
+        "the variant list did not parse out of: {message}"
+    );
+    names
+}
+
+/// The variant `value` **is**, read off its own serialization: a unit variant
+/// is a string, any other is a one-key object. Externally tagged, so both hold.
+pub fn name_of<T: Serialize>(value: &T) -> String {
+    match serde_json::to_value(value).expect("a wire type serializes") {
+        serde_json::Value::String(name) => name,
+        serde_json::Value::Object(map) => {
+            map.into_iter()
+                .next()
+                .expect("an externally tagged variant is a one-key object")
+                .0
+        }
+        other => panic!("not an externally tagged enum: {other}"),
+    }
+}
+
+/// Assert the cycle `walked` walks visits every variant `T` declares. The
+/// omission this catches is improvise-5ubs; §1 says why a count cannot.
+pub fn cycle_is_total<T: Serialize + DeserializeOwned>(walked: &[T], what: &str) {
+    let mut reached: Vec<String> = walked.iter().map(name_of).collect();
+    reached.sort();
+    reached.dedup();
+    let missed: Vec<String> = declared::<T>()
+        .into_iter()
+        .filter(|name| !reached.contains(name))
+        .collect();
+    assert!(
+        missed.is_empty(),
+        "{what}::after() never reaches {missed:?} — answering after() for a new \
+         variant is compiler-forced, pointing an existing arm AT it is not"
+    );
+}
+
+#[cfg(test)]
+mod tests {
+    /// The helper must refuse a message it did not parse rather than report an
+    /// empty list, which would pass every cycle. `context/code/protocol.md` §1.
+    #[test]
+    #[should_panic(expected = "serde no longer says")]
+    fn a_message_that_is_not_an_unknown_variant_is_refused() {
+        super::declared::<crate::Basis>();
+    }
+
+    /// Both externally tagged shapes name themselves, so a struct variant is
+    /// not silently skipped by [`super::cycle_is_total`].
+    #[test]
+    fn a_unit_variant_and_a_struct_variant_both_name_themselves() {
+        assert_eq!(super::name_of(&crate::AppMode::Normal), "Normal");
+        assert_eq!(super::name_of(&crate::AppMode::editing()), "Editing");
+    }
+}