diff --git a/context/code/command.md b/context/code/command.md index 26b5b25d..7ecb5805 100644 --- a/context/code/command.md +++ b/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::()` 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 diff --git a/context/test-evidence/command.md b/context/test-evidence/command.md index 3e2d3e6e..8be758b2 100644 --- a/context/test-evidence/command.md +++ b/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 diff --git a/context/test-evidence/tooling.md b/context/test-evidence/tooling.md index 01148f92..223bdddd 100644 --- a/context/test-evidence/tooling.md +++ b/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. diff --git a/crates/improvise-command/src/ui/effect.rs b/crates/improvise-command/src/ui/effect.rs index 1739ace8..6f300035 100644 --- a/crates/improvise-command/src/ui/effect.rs +++ b/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" ); } diff --git a/crates/improvise-command/src/ui/reduce.rs b/crates/improvise-command/src/ui/reduce.rs index 13386735..7b00f3eb 100644 --- a/crates/improvise-command/src/ui/reduce.rs +++ b/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 { vec![ ModelCommand::SetCell { diff --git a/scripts/check-comment-runs.py b/scripts/check-comment-runs.py index a6c0f76d..6f34f27f 100755 --- a/scripts/check-comment-runs.py +++ b/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",