Delete the invented Command::ClearBuffer, and guard the class

improvise-m0sx. `Command::ClearBuffer(String)` was declared on the wire and
composed by nothing in production: its only occurrences were the enum, the
fold arm, and two of reduce.rs's own tests. Its fold arm removed the buffer
key, while the only two implementations of its own name — the host's
`clear-buffer` command and resolve.rs::write_buffer — both compose
`SetBuffer { value: "" }`, which inserts. So the variant was invented rather
than derived and disagreed with both ends.

Deleted rather than given an effect. Adding a ClearBuffer effect to justify
the variant reverses decision(improvise-cqi, command_set_derived_not_invented),
and it would buy nothing: both readers of the map end in unwrap_or_default(),
so an absent key and an empty one are indistinguishable to every reader.

The guard is the generalisation. vocabulary_is_composed.rs reads the variant
list off serde's derive — the one enumeration the compiler maintains — and
requires each name to be constructed in non-test source, or named in a short
NOT_COMPOSED list with its verdict, checked in both directions.

It found two more orphans nobody had connected to the vocabulary, both
already filed as resolver gaps rather than inventions: SetYanked
(improvise-lut7) and SetSearchMode (improvise-fo0w item 1).

Proved discriminating twice: removing SetTileCatIdx's three production
composers failed the first test naming it, and listing the composed
SetBuffer in NOT_COMPOSED failed the second.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
Edward Langley
2026-09-22 19:59:41 -07:00
parent e7de19d011
commit 21184bbf64
5 changed files with 335 additions and 7 deletions
+45
View File
@@ -182,6 +182,51 @@ Two derivation traps worth keeping:
`MarkDirty` is gone; `App::apply_effects` now raises the flag off
`EffectKind::Model(_)` directly.
### The derivation runs backwards too, and `ClearBuffer` was the proof
A derivation checked only forwards — *does every effect have a command* —
misses the inverse: **a command with no effect**. `Command::ClearBuffer(String)`
was one. Nothing in `improvise-command` is named `ClearBuffer`; the host's
`clear-buffer` command composes `core::write_buffer(name, String::new())`,
which is `effect::SetBuffer` and therefore **inserts**, and `resolve.rs`'s
`write_buffer` composes the same `Command::SetBuffer`. The wire variant
**removed** the key instead. So the one behaviour both ends actually implement
disagreed with the variant that names it (improvise-m0sx).
It was deleted rather than given an effect, for three reasons.
- It is not in the derivation's image. Adding a `ClearBuffer` *effect* to
justify the variant reverses the rule — the wire would become the source of
truth for host semantics, which is the direction
`decision(improvise-cqi, command_set_derived_not_invented)` exists to forbid.
- It bought nothing. Both readers of the map — `cmd::core::read_buffer` and
`resolve.rs`'s reader — end in `unwrap_or_default()`, so an absent key and an
empty one are **indistinguishable to every reader**. The only observable
difference was `ViewState` equality between a folded client and a
snapshot-seeded one, and deleting the variant settles that equally, by
leaving exactly one way to clear.
- Nothing composed it. Its only three occurrences were the definition, the
fold arm, and two of `reduce.rs`'s own tests — which **pinned the divergent
behaviour**, asserting the key was absent. A test can hold a defect in place.
**A variant nothing composes is either dead or a promise the protocol does not
keep**, and that is the hazard the deletion generalises:
`vocabulary_is_composed.rs` reads the variant list off serde's derive — the one
enumeration the compiler maintains — and requires each name to be constructed
somewhere in non-test source, or to appear in a short `NOT_COMPOSED` list with
its verdict. The list is checked in **both** directions, so an entry that starts
being composed fails just as loudly as one that stops
(`doctrine(a_stale_list_of_what_is_missing_is_worse_than_no_list)`).
Two limits are worth stating, because the check is a source scan.
`#[cfg(test)]` items are dropped by a line scan that relies on rustfmt putting
an item's closing brace at the attribute's own indentation; over-stripping would
**fail** the test rather than pass it, which is the safe direction. And a
`Command::X` inside `matches!` or `if let` is read as a construction rather than
a pattern, which can only make the check *weaker* for that variant — never raise
a false alarm. A variant nothing composes at all is immune to both, since its
only pattern occurrence is its own fold arm, which `=>` identifies.
### Why two model effects are key-path only
A command on this wire is routed by `Sessions::apply` with **no
+61
View File
@@ -908,3 +908,64 @@ Fault injected: resolving `items` off `axis.items` directly instead of through
`cycle()`. The Filter arm then indexes an empty `Some(vec![])` for a Page axis
with no items — and, more to the point, the `items.is_some()` spelling is the
one gotcha 27 says is *not* the split.
### vocabulary_is_composed::every_command_variant_is_composed_by_production_code
improvise-m0sx. `Command::ClearBuffer(String)` was declared on the wire and
composed by **nothing** in production: its only occurrences were the enum, the
fold arm, and two of `reduce.rs`'s own tests. Worse, its fold arm
(`view.buffers.remove(name)`) contradicted the only implementation of its own
name — `clear-buffer` composes `effect::SetBuffer { value: "" }`, which
*inserts*, and `resolve.rs::write_buffer` composes the same `Command::SetBuffer`.
So the variant was invented rather than derived, and it disagreed with both
ends. It is deleted; see `context/code/protocol.md` §2 for why deletion beat
giving it an effect.
This test is the guard for the class. It reads the variant list off serde's
`unknown variant` message (`variants::declared`) — the one enumeration the
compiler maintains — and scans non-test workspace source for each name in a
value position.
Fault injected: `Command::SetTileCatIdx` rewritten to `Command::SetRowOffset` at
its three production sites in `resolve.rs`, leaving only its test occurrences.
The test then failed naming `["ClearBuffer", "SetTileCatIdx"]` — i.e. it failed
the *property*, not its own setup, and named precisely the variant that lost its
composer. Reverted.
Why it cannot pass vacuously: `the_scan_reaches_production_code_at_all` fails if
the source walk returns nothing recognisable, which is the direction a broken
walk fails in (a broken walk otherwise reads as "everything is composed" only
for the *second* test, never this one — this one gets stricter, not weaker, when
the scan finds less).
### vocabulary_is_composed::the_uncomposed_list_names_nothing_that_is_composed
The other direction of the same list. `NOT_COMPOSED` names two variants that are
genuinely derived — `effect::SetYanked` and `effect::SetSearchMode` both exist —
but that no resolver composes yet, so they are resolver gaps rather than
inventions: improvise-lut7 (nothing resolves yank, so `p` reads a permanently
empty slot) and improvise-fo0w item 1 (search, deliberately held back until
search-navigate can ship with it).
Both are open and one is in progress, so the entries are expected to be removed.
This test is what forces that: an allowlist entry whose variant *is* composed
fails, so closing either ticket without deleting its line breaks the build
rather than leaving a stale claim. `the_uncomposed_list_names_only_real_variants`
covers the third direction — an entry naming no variant at all, which a rename
would produce.
Fault injected: adding `"SetBuffer"` to `NOT_COMPOSED`. `SetBuffer` is composed
in `resolve.rs`, so the test failed naming it.
### reduce::tests::buffers_are_set_and_cleared_by_name
Rewritten with improvise-m0sx. It previously folded `Command::ClearBuffer` and
asserted `!view.buffers.contains_key("edit")` — pinning the divergence, since
neither implementation of "clear this buffer" removes the key. It now folds
`SetBuffer { value: "" }`, which is what both ends compose, and asserts the key
is **present and empty**.
That assertion is the discriminating one: a fold arm that removed the key would
give `None` and fail. Asserting only that `"command"` survived would pass under
either behaviour, which is what the old spelling amounted to for the cleared
key.
+2
View File
@@ -16,6 +16,8 @@ pub mod resolve;
#[cfg(any(test, feature = "test-support"))]
pub mod variants;
pub mod viewmodel;
#[cfg(test)]
mod vocabulary_is_composed;
pub use basis::{Basis, BasisCache, Rejection};
pub use client::{ClientState, Sent};
+9 -7
View File
@@ -32,11 +32,12 @@ pub enum Command {
SetYanked(Option<CellValue>),
SetSearchQuery(String),
SetSearchMode(bool),
/// Write a named buffer. Clearing one is `value: ""` — there is no
/// `ClearBuffer`, because there is no such effect (improvise-m0sx, §2).
SetBuffer {
name: String,
value: String,
},
ClearBuffer(String),
SetTileCatIdx(usize),
SetPanelOpen {
panel: Panel,
@@ -266,9 +267,6 @@ pub fn reduce_view(view: &mut ViewState, cmd: &Command) {
Command::SetBuffer { name, value } => {
view.buffers.insert(name.clone(), value.clone());
}
Command::ClearBuffer(name) => {
view.buffers.remove(name);
}
Command::SetTileCatIdx(i) => view.tile_cat_idx = *i,
Command::SetPanelOpen { panel, open } => *view.panel_open_mut(*panel) = *open,
Command::SetPanelCursor { panel, cursor } => *view.panel_cursor_mut(*panel) = *cursor,
@@ -391,6 +389,8 @@ mod tests {
assert_eq!(clamped.help_page, last, "out of range clamps, never panics");
}
/// Clearing leaves the key **present and empty**, which is what both
/// implementations do. `context/test-evidence/protocol.md`.
#[test]
fn buffers_are_set_and_cleared_by_name() {
let view = fold(&[
@@ -402,10 +402,13 @@ mod tests {
name: "command".into(),
value: "w".into(),
},
Command::ClearBuffer("edit".into()),
Command::SetBuffer {
name: "edit".into(),
value: String::new(),
},
]);
assert_eq!(view.buffers.get("command").map(String::as_str), Some("w"));
assert!(!view.buffers.contains_key("edit"));
assert_eq!(view.buffers.get("edit").map(String::as_str), Some(""));
}
/// Both ends fold the same commands (improvise-m91); see
@@ -462,7 +465,6 @@ mod tests {
name: "edit".into(),
value: "v".into(),
},
Command::ClearBuffer("edit".into()),
Command::SetTileCatIdx(2),
Command::SetPanelOpen {
panel: Panel::View,
@@ -0,0 +1,218 @@
//! Every `Command` the derive declares is composed by production code, or is
//! named below with its verdict. Reasoning: `context/code/protocol.md` §2.
use std::collections::BTreeSet;
use std::path::{Path, PathBuf};
use crate::reduce::Command;
/// A variant production composes nowhere, and why. Checked in **both**
/// directions, so a composed entry fails here too (improvise-m0sx).
const NOT_COMPOSED: &[(&str, &str)] = &[
(
"SetYanked",
"improvise-lut7: nothing resolves yank, so paste reads a permanently \
empty slot. Derived — effect::SetYanked exists — so this is a resolver \
gap, not an invention.",
),
(
"SetSearchMode",
"improvise-fo0w item 1: derived from effect::SetSearchMode, and \
deliberately held back because search without search-navigate is a \
worse failure than the present silence.",
),
];
#[test]
fn every_command_variant_is_composed_by_production_code() {
let composed = composed_in_production();
let missing: Vec<String> = crate::variants::declared::<Command>()
.into_iter()
.filter(|name| !composed.contains(name))
.filter(|name| !NOT_COMPOSED.iter().any(|(n, _)| n == name))
.collect();
assert!(
missing.is_empty(),
"Command::{missing:?} is declared and composed by nothing in production. \
A variant nothing composes is either dead or a promise the protocol does \
not keep — name it in NOT_COMPOSED with its verdict, or delete it."
);
}
#[test]
fn the_uncomposed_list_names_nothing_that_is_composed() {
let composed = composed_in_production();
let stale: Vec<&str> = NOT_COMPOSED
.iter()
.map(|(name, _)| *name)
.filter(|name| composed.contains(*name))
.collect();
assert!(
stale.is_empty(),
"Command::{stale:?} is listed as uncomposed and IS composed. \
doctrine(a_stale_list_of_what_is_missing_is_worse_than_no_list)"
);
}
/// The list is checked against the derive, so an entry naming no variant at
/// all — a typo, or a variant since renamed — cannot sit here unread.
#[test]
fn the_uncomposed_list_names_only_real_variants() {
let declared = crate::variants::declared::<Command>();
let unknown: Vec<&str> = NOT_COMPOSED
.iter()
.map(|(name, _)| *name)
.filter(|name| !declared.iter().any(|d| d == name))
.collect();
assert!(
unknown.is_empty(),
"no Command variant is named {unknown:?}"
);
}
/// The scan must find *something*, or a broken walk reads as a clean bill of
/// health — the failure direction `check_the_checker` exists for.
#[test]
fn the_scan_reaches_production_code_at_all() {
let composed = composed_in_production();
assert!(
composed.contains("SetSelected") && composed.contains("Project"),
"the source walk found no production composer for even SetSelected or \
Project, so it is broken rather than reporting: {composed:?}"
);
}
/// Every `Command::<Name>` in a **value** position, in non-test source.
fn composed_in_production() -> BTreeSet<String> {
let mut found = BTreeSet::new();
for file in production_sources() {
let text = std::fs::read_to_string(&file).expect("a listed source reads");
for name in constructed_in(&strip_test_items(&text)) {
found.insert(name);
}
}
found
}
/// Rust under `src/`, workspace-wide. `tests/` is excluded by construction:
/// a variant only a test composes is exactly what this file looks for.
fn production_sources() -> Vec<PathBuf> {
let root = Path::new(env!("CARGO_MANIFEST_DIR"))
.parent()
.and_then(Path::parent)
.expect("crates/<crate> sits two levels under the workspace root")
.to_path_buf();
let mut roots = vec![root.join("src")];
for entry in std::fs::read_dir(root.join("crates")).expect("crates/ is readable") {
roots.push(entry.expect("a crates/ entry reads").path().join("src"));
}
let mut files = Vec::new();
for dir in roots {
collect_rust(&dir, &mut files);
}
files
}
fn collect_rust(dir: &Path, into: &mut Vec<PathBuf>) {
let Ok(entries) = std::fs::read_dir(dir) else {
return;
};
for entry in entries.flatten() {
let path = entry.path();
match path.is_dir() {
true => collect_rust(&path, into),
false if path.extension().is_some_and(|e| e == "rs") => into.push(path),
false => {}
}
}
}
/// Drop every `#[cfg(test)]` item. rustfmt puts an item's closing brace at the
/// attribute's own indentation, which is what makes this a line scan (§2).
fn strip_test_items(text: &str) -> String {
let lines: Vec<&str> = text.lines().collect();
let mut kept = String::new();
let mut i = 0;
while i < lines.len() {
let line = lines[i];
match line.trim_start().starts_with("#[cfg(test)]") {
false => {
kept.push_str(line);
kept.push('\n');
i += 1;
}
true => {
let indent = line.len() - line.trim_start().len();
let close = format!("{}}}", " ".repeat(indent));
i += 1;
while i < lines.len() && lines[i] != close {
i += 1;
}
i += 1;
}
}
}
kept
}
/// `Command::<Name>` occurrences that are not `match` patterns. A pattern is
/// followed by `=>`, `|` or `if` once its payload is skipped (§2).
fn constructed_in(text: &str) -> Vec<String> {
let bytes = text.as_bytes();
let mut found = Vec::new();
for (at, _) in text.match_indices("Command::") {
if at > 0 && is_word_byte(bytes[at - 1]) {
continue;
}
let name_at = at + "Command::".len();
let name: String = text[name_at..]
.chars()
.take_while(char::is_ascii_alphanumeric)
.collect();
if name.is_empty() {
continue;
}
let after = skip_payload(text, name_at + name.len());
if !is_pattern_tail(&text[after..]) {
found.push(name);
}
}
found
}
fn is_word_byte(b: u8) -> bool {
b.is_ascii_alphanumeric() || b == b'_'
}
/// Past a variant's `(..)` or `{..}` payload, if it has one. Braces and parens
/// only — a payload holding a `"("` in a string would stop this short.
fn skip_payload(text: &str, from: usize) -> usize {
let bytes = text.as_bytes();
let mut at = from;
while at < bytes.len() && (bytes[at] == b' ' || bytes[at] == b'\n') {
at += 1;
}
let (open, close) = match bytes.get(at) {
Some(b'(') => (b'(', b')'),
Some(b'{') => (b'{', b'}'),
_ => return from,
};
let mut depth = 0usize;
while at < bytes.len() {
if bytes[at] == open {
depth += 1;
} else if bytes[at] == close {
depth -= 1;
if depth == 0 {
return at + 1;
}
}
at += 1;
}
from
}
fn is_pattern_tail(rest: &str) -> bool {
let rest = rest.trim_start();
rest.starts_with("=>") || rest.starts_with('|') || rest.starts_with("if ")
}