Merge remote-tracking branch 'gh/main' into fix/lxg0-w1bu
# Conflicts: # context/test-evidence/command.md
This commit is contained in:
@@ -3267,3 +3267,84 @@ key the two questions agree, because a cursor key never carries a None-axis
|
||||
category. The **collapsed-group fold** half of improvise-l4mp is not covered:
|
||||
its key carries the group's name as an ordinary coordinate, so no question about
|
||||
the key alone can see it — it needs `GridLayout::folds_a_group`.
|
||||
|
||||
## A command is a function of its context — the determinism law (improvise-hzjo)
|
||||
|
||||
`cmd/determinism.rs` holds the law the old `law_toggle_group_involution` claimed
|
||||
and could not state. For **every** registered command and every argument list
|
||||
that builds it, the effects `execute` returns must be equal, by `Debug` text,
|
||||
across contexts built **independently from equal data**: four `App`s, each
|
||||
parsed from `web/demo.improv` and given the same buffers, expanded categories,
|
||||
yank, panels and cursor.
|
||||
|
||||
**Why the instances must be distinct.** The old test called `execute` twice
|
||||
against one `CmdContext`. Every `HashMap` it reached was the same instance with
|
||||
the same `RandomState`, so it iterated the same way both times: `f(x) == f(x)`,
|
||||
which nothing can fail. Separately built `App`s give every map its own seed. So a
|
||||
command whose effects follow hash iteration order disagrees with itself. Hash
|
||||
order is the one kind of nondeterminism this repo has actually shipped (gotcha
|
||||
18). This law can see it where a command's `execute` builds it, as the drill
|
||||
snapshot's record order was. It cannot see the other shipped instance, the
|
||||
analyzer's `HashSet`: `analyze_records` is reached only from inside
|
||||
`Effect::apply`, which is outside this law (see below). The companion test
|
||||
`only_distinct_instances_can_see_a_command_that_follows_hash_order` keeps that
|
||||
property checked: a test-only command that emits `ctx.buffers` in iteration
|
||||
order must agree with itself on one context and disagree across two.
|
||||
|
||||
**Four instances, not two.** A rival that takes *the first* entry of a map, not
|
||||
all of them, agrees across two seeds whenever both happen to put the same entry
|
||||
first. With 21 buffers that is roughly one chance in 21, and three comparisons
|
||||
make it negligible. The fixture's values are also chosen to stay distinct under
|
||||
the edits commands make. A first version used `v0`…`v15`, which `pop()` collapses
|
||||
to `v`, and so it let `command-mode-backspace` reading an arbitrary buffer pass.
|
||||
|
||||
**The store needs several records to order, so the drills come from the
|
||||
document (improvise-e1sm).** The registry's `candidate_args` are extended with
|
||||
names the demo really has, because coordinates that match nothing never reach
|
||||
the records a drill or a fit would order. A hand-picked name is not enough. The
|
||||
first version's only real drill was `Product/Latte, Quarter/Q2`: one record, two
|
||||
cells. With the store's sort removed, two contexts put those two cells in the
|
||||
same order about half the time, and with three comparisons the law passed about
|
||||
one run in eight (measured: 46 of 400). So the arguments now include **every
|
||||
item of every regular category as a one-coordinate drill**, read off the parsed
|
||||
document. A one-coordinate drill spans every other category whole, so it matches
|
||||
the whole slice of records under that item, not one record picked by name. In
|
||||
the demo that is four records and eight cells for each of the eight drills. The
|
||||
test
|
||||
also asserts that the widest candidate it actually passes matches at least four
|
||||
records. That guard catches a demo with too little data, and it catches the
|
||||
derived drills being dropped from the argument list. It does **not** catch the
|
||||
comparison losing its wide drills some other way, because it measures what the
|
||||
argument lists match, not what the built commands ordered (improvise-b9f2). The
|
||||
review showed this with a rival: a `drill-into-cell` parser that refuses fewer
|
||||
than two arguments sends every one-coordinate drill to the one-record cursor
|
||||
fallback. The guard still reports four records, and with the store's sort
|
||||
removed the law passed 38 of 300 runs. With the store's sort removed and the
|
||||
parser intact, the law fails 400 of 400 runs.
|
||||
|
||||
**What it cannot see.** Only commands and arguments the fixture reaches. A
|
||||
command whose nondeterminism needs some other state, such as records mode, a
|
||||
wizard or a drill in progress, is outside this fixture. And `Debug` equality is
|
||||
text equality: two effects that differ only in something their `Debug` omits are
|
||||
equal here.
|
||||
|
||||
**Its scope is what `execute` returns, not what `Effect::apply` does.** The law
|
||||
compares the effects a command hands back. Many effects are unit structs that
|
||||
decide all their work inside `apply`: `ApplyAndClearDrill`,
|
||||
`SortData`, `CleanEmptyRecords`, `Save`, `RebuildLayout`, `AcceptWizardModel`.
|
||||
Hash-order iteration inside an `apply` is therefore invisible here, however
|
||||
often the law runs. improvise-uo2p is the worked example.
|
||||
`ApplyAndClearDrill::apply` walks `drill.pending_edits`, a `HashMap`, in hash
|
||||
order. A measure edit and a category rename staged on one row can then write a
|
||||
ghost record. The command that emits it passes this law, because the effect's
|
||||
`Debug` is just its name. So "the one kind of nondeterminism this repo has
|
||||
shipped" is guarded here only on the `execute` side. The `apply` side needs its
|
||||
own test per effect, or a law over `App` state after `apply_effects`.
|
||||
|
||||
**The exemption list.** `OUTSIDE_THE_CONTEXT` names commands that legitimately
|
||||
depend on something outside the context, such as a clock or an entropy source,
|
||||
each with its reason. It is empty: `fit-measure`'s seed is an argument with a
|
||||
fixed default, not a clock read. The list is checked in both directions. An
|
||||
unlisted command that diverges fails the test, and so does a listed one that
|
||||
does not diverge. A stale entry therefore cannot linger, and a new registration
|
||||
is covered the day it lands.
|
||||
|
||||
@@ -5340,6 +5340,71 @@ Rivals, injected, full workspace run:
|
||||
`the_tile_bar_route_into_records_mode_does_not_strand_the_user`. The second one
|
||||
is improvise-bd9o's own test of the route this decision keeps.
|
||||
|
||||
## `command/cmd/determinism.rs` — the determinism law over every command (improvise-hzjo)
|
||||
|
||||
The reasoning is in `context/code/command.md`, "A command is a function of its
|
||||
context". This entry records what each test can see.
|
||||
|
||||
### every_command_gives_equal_effects_in_independently_built_equal_contexts
|
||||
|
||||
It runs all 89 registrations, 1500+ (command, args) pairs, in four contexts built
|
||||
from four separately parsed copies of `web/demo.improv`, and requires equal
|
||||
effect `Debug` text. It passed at `ae96b8c` with no exemptions. It is not a
|
||||
regression test for a live bug. It restores a law, and the evidence is which
|
||||
rivals it catches:
|
||||
|
||||
- **`command-mode-backspace` reading `ctx.buffers.values().next()`** instead of
|
||||
the `command` buffer: a real command following hash order. Injected, full
|
||||
workspace run: this test is the **only** failure, because every
|
||||
`text_buffer.rs` test builds a one-entry buffer map. With the comparison
|
||||
switched to one reused context (`compare(&a, from_ref(&a))`) the same rival
|
||||
**passes**. That is the half the ticket required: the distinct construction
|
||||
is what does the work.
|
||||
- **`DataStore::matching_positions` without its sort**: gotcha 18 re-shipped.
|
||||
This entry first said the test catches it through `drill-into-cell` and
|
||||
`fit-measure`'s record order. That was wrong (improvise-e1sm). `fit-measure`
|
||||
never diverged, in 60 failing runs in review and in 354 here. And the catch
|
||||
was a matter of luck. The only real drill was `Product/Latte, Quarter/Q2`,
|
||||
which is one record with two cells, so each other context agrees with
|
||||
probability 1/2. With the sort removed and the test binary run in a loop, the
|
||||
law **passed 46 of 400 runs** at `fc0b13c`, and `drill-into-cell
|
||||
["Product/Latte", "Quarter/Q2"]` was the only pair that ever diverged. The
|
||||
arguments now include every item of every regular category as a
|
||||
one-coordinate drill, read off the parsed document (four records, eight cells
|
||||
each in the demo). With the same injection the law **passed 0 of 400**, and
|
||||
all eight new drills diverged in about 1195 of 1200 pairwise comparisons each.
|
||||
Without the injection it passed 400 of 400. The three `cell.rs` order tests
|
||||
still pin the store directly.
|
||||
- **The derived drills dropped from the argument list** (the
|
||||
`all.extend(one_coordinate_drills(model))` line commented out): the test
|
||||
fails before comparing anything, with "the widest drill orders 1 records".
|
||||
That is the only way of losing the wide drills the guard can see. It measures
|
||||
what the argument lists match, not what the built commands ordered, so it can
|
||||
pass while the comparison has lost its wide drills (improvise-b9f2). Review
|
||||
rival: `drill-into-cell`'s parser changed to refuse fewer than two arguments,
|
||||
which sends every one-coordinate drill to the one-record cursor fallback. The
|
||||
guard stays silent and, with gotcha 18 re-injected, the law passed 38 of 300
|
||||
runs.
|
||||
- **A stale exemption**: `("quit", "stale")` listed in `OUTSIDE_THE_CONTEXT`.
|
||||
It fails as "listed … but did not".
|
||||
|
||||
**A vacuity this test itself had on its first version.** The fixture's filler
|
||||
buffers held `v0`…`v15`. The `command-mode-backspace` rival pops a character,
|
||||
so every value collapsed to `v` (or `v1`), and the rival passed across two
|
||||
distinct contexts. This is the "two derivations the fixture makes agree" shape.
|
||||
The values are now `value {i} of 16`. The comparison was also widened from one
|
||||
other instance to three, because a take-the-first rival agrees across two seeds
|
||||
about one time in 21.
|
||||
|
||||
### only_distinct_instances_can_see_a_command_that_follows_hash_order
|
||||
|
||||
The check-the-checker behind the law. A test-only `EmitsInHashOrder` emits one
|
||||
`SetBuffer` per `ctx.buffers` entry in iteration order. It must equal itself on
|
||||
one context, which is why the old single-context law was vacuous, and differ
|
||||
across two independently built ones. If a future refactor made the fixture
|
||||
share one map, or share one `RandomState`, the second assertion fails. Without
|
||||
this test the law would then pass everything and look healthy.
|
||||
|
||||
### a_fresh_session_has_no_history_to_step_back_through
|
||||
|
||||
improvise-lxg0. `project.rs::fresh_view` built a new session's slice by folding
|
||||
|
||||
@@ -0,0 +1,233 @@
|
||||
//! A command is a function of its context: two contexts built independently from
|
||||
//! equal data give equal effects — `context/code/command.md`, improvise-hzjo.
|
||||
|
||||
use std::collections::{BTreeSet, HashSet};
|
||||
|
||||
use super::core::{Cmd, CmdContext};
|
||||
use super::registry::tests::candidate_args;
|
||||
use crate::command::key::{KeyCode, KeyModifiers};
|
||||
use crate::model::Model;
|
||||
use crate::model::cell::{CellKey, CellValue};
|
||||
use crate::model::record::record_of;
|
||||
use crate::ui::app::App;
|
||||
use crate::ui::effect::{self, Effect};
|
||||
|
||||
/// Commands whose effects depend on something outside the context, with why.
|
||||
const OUTSIDE_THE_CONTEXT: &[(&str, &str)] = &[];
|
||||
|
||||
const DOCUMENT: &str = include_str!("../../../../../web/demo.improv");
|
||||
|
||||
/// One `App` over the shipped demo. Every context map holds several entries that
|
||||
/// stay distinct under an edit like `pop()` — see the evidence.
|
||||
fn fixture() -> App {
|
||||
let workbook = crate::persistence::parse_md(DOCUMENT).expect("the demo parses");
|
||||
let mut app = App::new(workbook, None);
|
||||
let vs = &mut app.view_state;
|
||||
for i in 0..16 {
|
||||
vs.buffers
|
||||
.insert(format!("b{i}"), format!("value {i} of 16"));
|
||||
}
|
||||
for (name, text) in [
|
||||
("edit", "42"),
|
||||
("formula", "Margin = Profit / Revenue"),
|
||||
("category", "Region"),
|
||||
("command", "add-item Product Mocha"),
|
||||
("export", "/tmp/improvise-hzjo.csv"),
|
||||
] {
|
||||
vs.buffers.insert(name.into(), text.into());
|
||||
}
|
||||
for cat in ["Product", "Quarter", "_Measure", "_Index", "_Dim"] {
|
||||
vs.expanded_cats.insert(cat.into());
|
||||
}
|
||||
vs.yanked = Some(CellValue::Number(7.0));
|
||||
vs.formula_panel_open = true;
|
||||
vs.category_panel_open = true;
|
||||
vs.view_panel_open = true;
|
||||
vs.cat_panel_cursor = 2;
|
||||
vs.view_panel_cursor = 1;
|
||||
app.cursor_mut().selected = (1, 1);
|
||||
app
|
||||
}
|
||||
|
||||
/// Every item as a one-coordinate drill: each spans the other categories whole,
|
||||
/// so the store has several records to order — see the evidence.
|
||||
fn one_coordinate_drills(model: &Model) -> Vec<Vec<String>> {
|
||||
let mut drills = Vec::new();
|
||||
for cat in model.regular_category_names() {
|
||||
for item in model.category(cat).expect("listed").ordered_item_names() {
|
||||
drills.push(vec![format!("{cat}/{item}")]);
|
||||
}
|
||||
}
|
||||
drills
|
||||
}
|
||||
|
||||
/// How many distinct records the widest all-`Cat/Item` candidate matches.
|
||||
fn widest_drill(model: &Model, candidates: &[Vec<String>]) -> usize {
|
||||
let coordinates = |args: &Vec<String>| -> Option<Vec<(String, String)>> {
|
||||
args.iter()
|
||||
.map(|a| {
|
||||
a.split_once('/')
|
||||
.map(|(c, i)| (c.to_string(), i.to_string()))
|
||||
})
|
||||
.collect()
|
||||
};
|
||||
candidates
|
||||
.iter()
|
||||
.filter(|args| !args.is_empty())
|
||||
.filter_map(coordinates)
|
||||
.map(|partial| {
|
||||
let cells = model.data.matching_cells(&partial);
|
||||
let records: HashSet<CellKey> = cells.iter().map(|(k, _)| record_of(k)).collect();
|
||||
records.len()
|
||||
})
|
||||
.max()
|
||||
.unwrap_or(0)
|
||||
}
|
||||
|
||||
/// The registry's candidates, plus names the demo really has: a drill or a fit
|
||||
/// over coordinates that match nothing never reaches the records it would order.
|
||||
fn args_the_document_names(model: &Model) -> Vec<Vec<String>> {
|
||||
let mut all = candidate_args();
|
||||
all.extend(one_coordinate_drills(model));
|
||||
all.extend(
|
||||
[
|
||||
vec!["Revenue"],
|
||||
vec!["Product/Latte", "Quarter/Q2"],
|
||||
vec!["Product/Latte", "Quarter/Q2", "_Measure/Cost"],
|
||||
vec![
|
||||
"editing",
|
||||
"5",
|
||||
"Product/Latte",
|
||||
"Quarter/Q2",
|
||||
"_Measure/Cost",
|
||||
],
|
||||
]
|
||||
.into_iter()
|
||||
.map(|v| v.into_iter().map(String::from).collect()),
|
||||
);
|
||||
all
|
||||
}
|
||||
|
||||
fn effects_of(name: &str, args: &[String], ctx: &CmdContext) -> Option<String> {
|
||||
let cmd = ctx.registry.interactive(name, args, ctx).ok()?;
|
||||
Some(format!("{:?}", cmd.execute(ctx)))
|
||||
}
|
||||
|
||||
/// What running every registered command under `a` and under each of `others` found.
|
||||
#[derive(Default)]
|
||||
struct Comparison {
|
||||
diverged: BTreeSet<&'static str>,
|
||||
shown: Vec<String>,
|
||||
never_built: Vec<&'static str>,
|
||||
compared: usize,
|
||||
}
|
||||
|
||||
fn compare(a: &CmdContext, others: &[CmdContext]) -> Comparison {
|
||||
let mut found = Comparison::default();
|
||||
let candidates = args_the_document_names(a.model);
|
||||
for name in a.registry.names().collect::<Vec<_>>() {
|
||||
let before = found.compared;
|
||||
for args in &candidates {
|
||||
let Some(left) = effects_of(name, args, a) else {
|
||||
continue;
|
||||
};
|
||||
found.compared += 1;
|
||||
for b in others {
|
||||
let right = effects_of(name, args, b).unwrap_or_default();
|
||||
if left != right {
|
||||
found.diverged.insert(name);
|
||||
found
|
||||
.shown
|
||||
.push(format!("{name} {args:?}:\n {left}\n {right}"));
|
||||
}
|
||||
}
|
||||
}
|
||||
if found.compared == before {
|
||||
found.never_built.push(name);
|
||||
}
|
||||
}
|
||||
found
|
||||
}
|
||||
|
||||
/// Evidence: `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn every_command_gives_equal_effects_in_independently_built_equal_contexts() {
|
||||
let apps: Vec<App> = (0..4).map(|_| fixture()).collect();
|
||||
let contexts: Vec<CmdContext> = apps
|
||||
.iter()
|
||||
.map(|app| app.cmd_context(KeyCode::Null, KeyModifiers::NONE))
|
||||
.collect();
|
||||
let model = contexts[0].model;
|
||||
let widest = widest_drill(model, &args_the_document_names(model));
|
||||
assert!(
|
||||
widest >= 4,
|
||||
"the widest drill orders {widest} records, so the store's order is nearly a coin toss"
|
||||
);
|
||||
let found = compare(&contexts[0], &contexts[1..]);
|
||||
assert!(
|
||||
found.never_built.is_empty(),
|
||||
"no candidate args build these in the fixture, so the law never ran them: {:?}",
|
||||
found.never_built
|
||||
);
|
||||
let listed: BTreeSet<&str> = OUTSIDE_THE_CONTEXT.iter().map(|(n, _)| *n).collect();
|
||||
let unlisted: Vec<&str> = found
|
||||
.shown
|
||||
.iter()
|
||||
.filter(|s| !listed.iter().any(|n| s.starts_with(&format!("{n} "))))
|
||||
.map(String::as_str)
|
||||
.collect();
|
||||
assert!(
|
||||
unlisted.is_empty(),
|
||||
"effects depend on more than the context ({} pairs compared):\n{}",
|
||||
found.compared,
|
||||
unlisted.join("\n")
|
||||
);
|
||||
let stale: Vec<&&str> = listed
|
||||
.iter()
|
||||
.filter(|n| !found.diverged.contains(**n))
|
||||
.collect();
|
||||
assert!(
|
||||
stale.is_empty(),
|
||||
"listed as depending on something outside the context, but did not: {stale:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Iterates a context `HashMap` and emits in that order: the one nondeterminism
|
||||
/// this repo has shipped (gotcha 18). The law must see it; a reused context cannot.
|
||||
#[derive(Debug)]
|
||||
struct EmitsInHashOrder;
|
||||
impl Cmd for EmitsInHashOrder {
|
||||
fn name(&self) -> &'static str {
|
||||
"emits-in-hash-order"
|
||||
}
|
||||
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
|
||||
ctx.buffers
|
||||
.iter()
|
||||
.map(|(name, value)| {
|
||||
Box::new(effect::SetBuffer {
|
||||
name: name.clone(),
|
||||
value: value.clone(),
|
||||
}) as Box<dyn Effect>
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
}
|
||||
|
||||
/// Evidence: `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn only_distinct_instances_can_see_a_command_that_follows_hash_order() {
|
||||
let (first, second) = (fixture(), fixture());
|
||||
let a = first.cmd_context(KeyCode::Null, KeyModifiers::NONE);
|
||||
let b = second.cmd_context(KeyCode::Null, KeyModifiers::NONE);
|
||||
let run = |ctx: &CmdContext| format!("{:?}", EmitsInHashOrder.execute(ctx));
|
||||
assert_eq!(
|
||||
run(&a),
|
||||
run(&a),
|
||||
"one reused context iterates its maps the same way twice"
|
||||
);
|
||||
assert_ne!(
|
||||
run(&a),
|
||||
run(&b),
|
||||
"two independently built contexts must iterate differently, or the law is vacuous"
|
||||
);
|
||||
}
|
||||
@@ -13,6 +13,9 @@ pub mod text_buffer;
|
||||
pub mod tile;
|
||||
pub mod wizard;
|
||||
|
||||
#[cfg(test)]
|
||||
mod determinism;
|
||||
|
||||
// Re-export items used by external code
|
||||
pub use self::core::{Cmd, CmdContext, CmdRegistry};
|
||||
pub use registry::default_registry;
|
||||
|
||||
@@ -498,13 +498,13 @@ pub fn default_registry() -> CmdRegistry {
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
pub(super) mod tests {
|
||||
use super::*;
|
||||
use crate::command::cmd::test_helpers::*;
|
||||
|
||||
/// Argument vectors to try when constructing a command, most specific
|
||||
/// first. A command is exercised by the first one its parser accepts.
|
||||
fn candidate_args() -> Vec<Vec<String>> {
|
||||
pub(in crate::command::cmd) fn candidate_args() -> Vec<Vec<String>> {
|
||||
[
|
||||
vec![],
|
||||
vec!["1"],
|
||||
|
||||
Reference in New Issue
Block a user