Review round 1: three checks that did not check what they named

All three found by the adversarial review of 019cbc1, each by injection.

1. every_model_command() is NOT compiler-maintained, and both this PR's prose
   and reduce.rs's own doc comment said it was. variant_name's exhaustive match
   forces an arm in variant_name, not an entry in the value list -- deleting
   ModelCommand::TogglePruneEmpty from it failed the new completeness check and
   NOTHING ELSE in the workspace. So the diff is two independent hand-written
   lists: strictly better than one list against its own length, and not the
   derived enumeration improvise-5ubs got for the two enums. Doc comment
   corrected; the prose in context/code/command.md and the evidence entry now
   say what it is. Closing it properly wants improvise_protocol::variants made
   pub so declared::<ModelCommand>() can bound the list -- another wave's crate,
   reported rather than taken.

   The assertion message was also ONE-DIRECTIONAL ("a name only on the right
   ..."), which is the shape that injection produces and which invites the wrong
   repair: delete the case on the left, degrading both sides together. It now
   names both directions and says which is which.

2. SELF_TEST_DUPLICATES case 3 was named for the ambiguity guard and never
   reached it: both its headings were module-qualified, so canonical() returned
   on the exact-match arm before touching by_leaf. Dropping the guard
   (`if matches` for `if len(matches) == 1`) left --self-test at exit 0. The
   fixture is now one BARE heading over an ambiguous leaf plus BOTH qualified
   spellings, so the injection collides with one of them whichever way
   next(iter(set)) falls -- deterministic across five runs, verified.

3. The evidence entry said five pre-existing records-mode tests fail under the
   settle-before-rebuild injection; measured, it is six. And "three hundred
   lines above" is four hundred.

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

context/code/command.md

@@ -1352,13 +1352,27 @@ adversarial review of PR #78 added a fourteenth `pub struct` declaring
 improvise-5ubs's answer — read serde's derive-generated variant list out of its
 `unknown variant … expected one of` message — cannot reach this one: an effect
 is a struct implementing a trait, so there is no derived enumeration of effects
-at all. But there is one of the **wire vocabulary**, and gotcha 38 says
-`ModelCommand` is derived from the model effects with `model_effects` mapping
-each variant back to the effect it came from. So the list is now diffed against
-the `LayoutGeometry` effects *reachable through `ModelCommand`*, which is an
-enumeration something else maintains, and the failure names the missing effect
-rather than a number. The two lists agree exactly today, which is itself the
-evidence the substitution lost nothing.
+at all. But gotcha 38 says `ModelCommand` is derived from the model effects,
+with `model_effects` mapping each variant back to the effect it came from, so
+the list is now diffed against the `LayoutGeometry` effects *reachable through
+`ModelCommand`*, and the failure names the missing effect rather than a number.
+The two lists agree exactly today, which is itself the evidence the
+substitution lost nothing.
+
+**The right-hand side is a second hand-written list, and saying otherwise was
+the first thing review caught** (PR #96). `every_model_command()`'s doc comment
+claimed the enumeration was compiler-forced — `variant_name` matches
+exhaustively, so a new variant fails the build. That inference is invalid: the
+build fails in *`variant_name`*, and nothing obliges the value list to gain an
+entry. Deleting one from it fails this check and nothing else in the workspace.
+So the diff is **two independent hand-written lists**, which is strictly better
+than one list against its own length and is not the compiler-maintained
+enumeration the ticket asked for. The assertion message therefore names **both**
+directions, because a short right-hand side otherwise reads as a spurious case
+and invites deleting the left — degrading both sides together, which is the
+failure the ticket exists to close. Closing it properly wants
+`improvise_protocol::variants` made `pub` so `declared::<ModelCommand>()` can
+bound the enumeration; that crate is another wave's ground and it is filed.
 
 **Its bound is stated rather than assumed**: this closes the effects on the
 wire, and an effect declaring `LayoutGeometry` that no `ModelCommand` names is

context/test-evidence/command.md

@@ -3531,7 +3531,8 @@ Driven in both directions, with the crossing asserted before and after so a
 fixture that stopped crossing fails rather than passes. Injected fault: move
 `settle_normal_mode()` back above `rebuild_layout()` at the end of
 `apply_effects` — the exact regression PR #91's round 2 fixed. This test fails
-by name; so does gap 1's, along with five pre-existing records-mode tests.
+by name; so does gap 1's, along with six pre-existing records-mode tests
+(measured in review; an earlier draft of this entry said five).
 
 **Scope note, stated because an overclaim is worse than a gap**: this drives
 the *effects* `SwitchViewAtCursor` emits, not the view-panel key path that
@@ -3549,11 +3550,22 @@ added a fourteenth `pub struct` declaring `LayoutGeometry`, pointed at by
 nothing, and the whole suite stayed green.
 
 improvise-5ubs's mechanism cannot reach it: an effect is a struct implementing
-a trait, so serde has no derived enumeration to diff against. The **wire
-vocabulary** does have one, and `model_effects` maps each `ModelCommand`
-variant back to the effect it came from (gotcha 38), so the list is diffed
-against the `LayoutGeometry` effects reachable through `ModelCommand`. The two
-sets are equal today, which is the evidence the substitution lost nothing.
+a trait, so serde has no derived enumeration to diff against. `model_effects`
+maps each `ModelCommand` variant back to the effect it came from (gotcha 38),
+so the list is diffed against the `LayoutGeometry` effects reachable through
+`ModelCommand`. The two sets are equal today, which is the evidence the
+substitution lost nothing.
+
+**Both sides are hand-written, and the right one is not compiler-maintained.**
+Review of PR #96 injected the deletion of `ModelCommand::TogglePruneEmpty` from
+`every_model_command()`: this was the **only** test in the workspace that
+failed, and `every_model_command`'s doc comment claiming the list was
+compiler-forced was wrong — `variant_name`'s exhaustive match forces an arm
+*there*, not an entry in the list. Two independent hand-written lists beat one
+list against its own length; they are not the derived enumeration
+improvise-5ubs got for the two enums. The assertion names **both** directions,
+because a short right-hand side otherwise reads as a spurious case on the left
+and invites deleting it — degrading both sides together.
 
 Injected fault: drop `TogglePruneEmpty` from `layout_geometry_cases()`. The old
 count said `12 != 13`; this says

context/test-evidence/tooling.md

@@ -201,7 +201,7 @@ improvise-hsoe. `--check-evidence` verified that a heading names a test that
 PR #92 shipped two entries under
 `a_layout_change_moves_the_layout_version_and_no_record`, and two more pairs
 sat in `tui.md` — one of them asserting that `THE_MEMBERSHIP_CORRECT_SPANS` was
-"a live tripwire, do not remove it" some three hundred lines above the entry
+"a live tripwire, do not remove it" some four hundred lines above the entry
 recording that improvise-gd83 had removed it. A reader meeting the older copy
 first takes it as current, which is worse than an orphan: an orphan announces
 itself as broken.

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

@@ -1431,8 +1431,10 @@ mod tests {
         listed.dedup();
         assert_eq!(
             listed, reachable,
-            "the LayoutGeometry case list and the wire vocabulary disagree; a name \
-             only on the right is an effect nothing here exercises"
+            "the LayoutGeometry case list and the wire vocabulary disagree. A name \
+             only on the LEFT means every_model_command() has a hole, not that the \
+             case is spurious; a name only on the RIGHT is an effect nothing here \
+             exercises. Read `context/code/command.md` before deleting either side"
         );
     }
 

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

@@ -492,8 +492,8 @@ pub(crate) mod tests {
         app
     }
 
-    /// One value per `ModelCommand` variant. [`variant_name`] matches
-    /// exhaustively, so a new variant fails the build until it is listed here.
+    /// One value per `ModelCommand` variant — by hand, and NOT compiler-forced:
+    /// [`variant_name`]'s exhaustive match only forces an arm there (PR #96).
     pub(crate) fn every_model_command() -> Vec<ModelCommand> {
         vec![
             ModelCommand::SetCell {

scripts/check-comment-runs.py

@@ -958,8 +958,10 @@ SELF_TEST_DUPLICATES = [
      "### a_real_test\n### m::tests::a_real_test\n", "m::tests::a_real_test: test\n", "",
      ["m::tests::a_real_test"]),
 
-    ("two DIFFERENT tests sharing a leaf are not duplicates, they are ambiguous",
-     "### m::tests::shared\n### n::tests::shared\n",
+    # A BARE heading, because two qualified ones resolve by the exact-match arm
+    # and never reach the ambiguity guard this case is named for (review, PR #96).
+    ("a bare heading over an ambiguous leaf is not a duplicate of a qualified one",
+     "### shared\n### m::tests::shared\n### n::tests::shared\n",
      "m::tests::shared: test\nn::tests::shared: test\n", "", []),
 
     ("a JS heading twice is refused too",