fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'main' into fix/3j70-config-witness

Browse Source
This commit is contained in:
Ed Langley
2026-09-24 01:57:16 -07:00
committed by GitHub
parent 12a92a8669 220abc61dd
commit 8f218644fe
53 changed files with 5107 additions and 748 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.agents/skills/pr-watch/SKILL.md
+5 -2
View File
@@ -12,12 +12,15 @@ scripts/pr-watch <pr-number> [--expect <sha>] [--timeout <sec>] [--interval <sec
```
exit 0 every check COMPLETED, none failed, and MERGEABLE with a state of
CLEAN, UNSTABLE or HAS_HOOKS
exit 1 a check failed, or the PR is not mergeable (CONFLICTING, BLOCKED,
DIRTY, BEHIND, DRAFT, or a state it does not know)
exit 1 a check failed (under any mergeability, BLOCKED included), or the
PR is not mergeable (CONFLICTING, DIRTY, BEHIND, DRAFT, or a state
it does not know)
exit 2 the head moved during the wait, or is not --expect
exit 3 timed out with checks still running, mergeability still UNKNOWN,
or still BLOCKED with every registered check green
exit 4 usage or environment error
exit 5 the PR is MERGED or CLOSED, at the head the watch pinned; it ends on
the first poll that sees it closed, which may be mid-wait
```
**Exit 0 now means mergeable, not only green.** GitHub computes mergeability
.beads/issues.jsonl
+301 -32
View File
File diff suppressed because one or more lines are too long
.github/actionlint.yaml
+6
View File
@@ -0,0 +1,6 @@
# Labels the self-hosted NixOS microVM runners register with.
self-hosted-runner:
labels:
- nixos
- microvm
- audited
.github/workflows/ci.yml
+85 -174
View File
@@ -12,6 +12,15 @@ on:
env:
CARGO_TERM_COLOR: always
# Every job runs on the self-hosted NixOS microVM runners, and every `run:`
# step inside the flake's `ci` dev shell. Generic-Linux downloads (rustup,
# setup-node's node, Playwright's browsers) cannot run on NixOS, so the
# toolchain comes from flake.nix, the same one `nix develop` gives locally.
# Each step re-enters the shell; after the first, the eval cache makes it cheap.
defaults:
run:
shell: nix develop .#ci --command bash --noprofile --norc -eo pipefail {0}
jobs:
# ---------------------------------------------------------------------------
# The one place wasm-pack runs (improvise CI dedup).
@@ -35,7 +44,7 @@ jobs:
# would serialise a 109s job behind a 126s one for no artifact at all.
build:
name: Build all bundles
runs-on: ubuntu-latest
runs-on: [self-hosted, nixos, microvm]
steps:
- uses: actions/checkout@v7
# First, because it needs nothing and takes a second: the gate that stops
@@ -44,61 +53,27 @@ jobs:
# below, so it fails under this job's name rather than another's.
- name: The stamp gate refuses what it claims to
run: ./scripts/checkers.sh assemble-stamp
- uses: dtolnay/rust-toolchain@stable
with:
targets: wasm32-unknown-unknown
# Two workspaces, because `harness/opfs-contract` carries an empty
# `[workspace]` table and so has its own target dir, which the default
# single-workspace configuration never cached (it recompiled its whole
# dependency tree every run).
- uses: Swatinem/rust-cache@v2
with:
workspaces: |
. -> target
harness/opfs-contract -> target
# One home for the two pins, so the release capture
# (scripts/build-web-release.sh, improvise-oujb) provisions the same
# versions rather than a second copy free to go stale. The grep keeps a
# comment or a blank line out of $GITHUB_ENV, which takes NAME=VALUE only.
- name: Pinned web toolchain versions
run: grep -E '^[A-Z_]+=' scripts/web-toolchain-versions.env >> "$GITHUB_ENV"
# Pinned release binary rather than `cargo install` (a three-minute
# source build) or a third-party action. wasm-pack runs its own bundled
# `wasm-opt`, which is why improvise-wasm-client's Cargo.toml names
# `--enable-bulk-memory`: without it the optimizer rejects the
# `memory.copy` LLVM emits regardless.
# v0.15.0 is what `nix develop` supplies, and the bundle was built and
# measured against it locally (346 KB, 120 KB gzipped). Pinning the same
# version is what makes that local run evidence about this one.
- name: Install wasm-pack
# Built by Nix (packages.web and packages.site in nix/ci.nix), which runs
# the same harness/build.sh and web/assemble.sh a developer runs, so there
# is still one answer to "how is the bundle built". `web` is the three
# wasm bundles plus `harness/serve`; `site` re-assembles from web's pkg
# dirs with the commit SHA as the cache-bust token (improvise-9x6), where
# the harness tree is stamped `harness`. Dependencies are their own
# derivations, substituted from titan's cache when Cargo.lock is unchanged.
- name: Build the bundles, the harness tree and the site
run: |
set -eu
url="https://github.com/rustwasm/wasm-pack/releases/download/${WASM_PACK_VERSION}/wasm-pack-${WASM_PACK_VERSION}-x86_64-unknown-linux-musl.tar.gz"
curl -sSfL "$url" | tar -xz --strip-components=1 -C /usr/local/bin \
"wasm-pack-${WASM_PACK_VERSION}-x86_64-unknown-linux-musl/wasm-pack"
wasm-pack --version
# Both assemblies below generate the prose site with node. The runner
# image ships one, but pinning it is what makes a local run evidence.
- uses: actions/setup-node@v4
with:
node-version: ${{ env.NODE_VERSION }}
# The same script a developer runs locally, so there is one answer to
# "how is the bundle built" rather than two that can drift. It builds all
# three wasm bundles (client, worker, opfs-contract) and assembles
# `harness/serve` — the deployed arrangement plus /opfs/.
- name: Build the bundles and the harness tree
run: ./harness/build.sh
# Re-assembles from the pkg dirs the step above already produced — no
# wasm-pack runs here. A second assembly rather than a copy of
# `harness/serve` because the cache-bust token differs: the harness tree
# is stamped `harness` and the published site the commit SHA, which is
# what makes a returning visitor pick up a fresh bundle (improvise-9x6).
- name: Assemble the site
run: ./web/assemble.sh site "$GITHUB_SHA"
nix build --print-build-logs --out-link result-web .#web
nix build --print-build-logs --out-link result-site .#site
# The upload steps below read these paths. Copied rather than linked:
# an artifact of a symlink into /nix/store is a dangling link elsewhere.
- name: Lay the outputs out where the uploads read them
run: |
rm -rf harness/serve web/pkg web/worker-pkg site
cp -rL result-web/harness-serve harness/serve
cp -rL result-web/pkg web/pkg
cp -rL result-web/worker-pkg web/worker-pkg
cp -rL result-site site
chmod -R u+w harness/serve web/pkg web/worker-pkg site
- run: find site -type f | sort
- run: du -sh site
@@ -133,27 +108,27 @@ jobs:
test:
name: Test and lint (host)
runs-on: ubuntu-latest
runs-on: [self-hosted, nixos, microvm]
steps:
- uses: actions/checkout@v7
- uses: dtolnay/rust-toolchain@stable
with:
components: clippy, rustfmt
- uses: Swatinem/rust-cache@v2
# NOT `cargo fmt --all`, which reaches only ONE of the two workspace
# roots. Exit 2 here is a usage error, never a verdict (improvise-b4ks).
- name: Every workspace root is formatted
run: ./scripts/fmt-workspaces.sh
- run: cargo clippy --workspace --all-targets -- -D warnings
- run: cargo test --workspace
# checks.clippy and checks.test in nix/ci.nix: the same cargo commands,
# on the dev/test profiles, with dependencies as a cached derivation.
- name: cargo clippy --workspace --all-targets -- -D warnings
run: nix build --print-build-logs .#checks.x86_64-linux.clippy
# Also runs the test-evidence gate (check-comment-runs.py
# --check-evidence), which lists the tests this derivation compiled.
- name: cargo test --workspace, and test-evidence headings name real tests
run: nix build --print-build-logs .#checks.x86_64-linux.test
# The comment cap and the test-evidence keys (improvise-9c8c); without
# this step the rule is advisory. See context/design-principles.md §6.
- name: The comment checker catches what it has been fixed for
run: python3 scripts/check-comment-runs.py --self-test
- name: Comment runs capped at two lines
run: python3 scripts/check-comment-runs.py
- name: Test-evidence headings name real tests
run: python3 scripts/check-comment-runs.py --check-evidence
# `pages-deploy` needs every other job, and saying so was not enough: a
# job added during review of PR #83 was ungated and nothing noticed.
- name: The deploy gate refuses what it claims to
@@ -161,83 +136,44 @@ jobs:
- name: Every check job gates the deploy
run: python3 scripts/check-ci-graph.py
# Too slow on GitHub Actions
#
# nix-test:
# name: Nix Test
# runs-on: ubuntu-latest
# steps:
# - name: Free up disk space
# run: |
# sudo rm -rf /usr/share/dotnet \
# /usr/local/lib/android
# sudo df -h
# - uses: actions/checkout@v7
# - name: Install Nix
# uses: DeterminateSystems/nix-installer-action@v14
# - name: Magic Nix Cache
# uses: DeterminateSystems/magic-nix-cache-action@v8
# - name: Check Nix health
# run: nix doctor
# - run: nix build
# ---------------------------------------------------------------------------
# The crate2nix build (Cargo.nix), which `cargo` never exercises. It was off
# because GitHub-hosted runners start with an empty store and rebuilt every
# dependency crate each run. The NixOS runners substitute from titan's
# harmonia first, and titan builds gh/main's `#improvise` hourly, so a PR that
# leaves Cargo.lock alone rebuilds only the workspace crates it changed.
nix-build:
name: Nix build (crate2nix)
runs-on: [self-hosted, nixos, microvm]
steps:
- uses: actions/checkout@v7
- name: nix build .#improvise
run: nix build --print-build-logs .#improvise
wasm:
name: wasm32 check (browser bundle crates)
runs-on: ubuntu-latest
runs-on: [self-hosted, nixos, microvm]
steps:
- uses: actions/checkout@v7
- uses: dtolnay/rust-toolchain@stable
with:
targets: wasm32-unknown-unknown
- uses: Swatinem/rust-cache@v2
# Only the crates that go into the browser bundle. The root `improvise`
# crate is the terminal binary and depends on crossterm, which does not
# build for wasm — that is correct, not a gap (improvise-ywd).
- name: improvise-core
run: cargo check -p improvise-core --target wasm32-unknown-unknown
- name: improvise-formula
run: cargo check -p improvise-formula --target wasm32-unknown-unknown
- name: improvise-protocol
run: cargo check -p improvise-protocol --target wasm32-unknown-unknown
- name: improvise-io
run: cargo check -p improvise-io --target wasm32-unknown-unknown
- name: improvise-command
run: cargo check -p improvise-command --target wasm32-unknown-unknown
# The browser client itself. This is the one crate for which a wasm
# build is the *point* rather than a property worth keeping, so a
# failure here is not a portability regression but a broken deliverable
# (improvise-gsw).
- name: improvise-wasm-client
run: cargo check -p improvise-wasm-client --target wasm32-unknown-unknown
# The worker half of the standalone deployment (improvise-9x6). Same
# terms as the client: a wasm build is the point, so a failure here is a
# broken deliverable rather than a portability regression.
- name: improvise-worker-server
run: cargo check -p improvise-worker-server --target wasm32-unknown-unknown
# Not in the browser bundle yet — nothing depends on improvise-gp. Gated
# anyway because it is a candidate (fitting a tree in the browser) and
# staying wasm-clean is free now, expensive to retrofit (improvise-z5eg).
- name: improvise-gp
run: cargo check -p improvise-gp --target wasm32-unknown-unknown
# checks.wasm-check in nix/ci.nix: `cargo check --target wasm32` for each
# crate in the browser bundle, one at a time, so features are not unified
# across them. The root `improvise` crate is the terminal binary and
# depends on crossterm, which does not build for wasm — that is correct,
# not a gap (improvise-ywd). The log names each crate as it is checked.
- name: cargo check --target wasm32-unknown-unknown, per bundle crate
run: nix build --print-build-logs .#checks.x86_64-linux.wasm-check
# ---------------------------------------------------------------------------
# The browser JavaScript's unit tests. Its own job, deliberately: a failure
# here is a decision `web/shell.js` or `web/serial.js` got wrong, and
# reporting that as "Browser harness" would put it behind an 80 MB browser
# download and two wasm-pack builds. It needs neither — `node --test` is in
# the runtime, and this job installs nothing at all.
# the runtime, which the dev shell supplies; this job installs nothing.
js:
name: Browser JS unit tests (node --test)
runs-on: ubuntu-latest
runs-on: [self-hosted, nixos, microvm]
steps:
- uses: actions/checkout@v7
- uses: actions/setup-node@v4
with:
node-version: 22
# The glob is the SCRIPT's, not the shell's and not node's. Under bash an
# unmatched glob survives as a literal, node globs it, matches nothing and
# exits 0 having run no test -- so this step gated nothing the moment
@@ -252,26 +188,26 @@ jobs:
kb:
name: Knowledge base consistency
runs-on: ubuntu-latest
runs-on: [self-hosted, nixos, microvm]
steps:
- uses: actions/checkout@v7
- uses: cachix/install-nix-action@v31
# kb.pl is executable knowledge, so it gets checked like code: rules
# must reject what they should, domains must close, and functional keys
# must not carry conflicting values. See kb_report/0 in the file.
# trealla comes from the dev shell, as it does locally.
- name: kb_consistent
run: |
nix shell nixpkgs#trealla --command tpl prolog-sessions/kb.pl \
tpl prolog-sessions/kb.pl \
-g "(kb_consistent -> halt(0) ; (kb_report, halt(1)))"
# scripts/kb is the only thing that writes kb.pl, so its refusals are as
# load-bearing as kb_consistent itself. Without this the replacement
# writer could regress green -- the review that found the directive
# bypass also found that nothing ran this suite.
- name: The kb tool refuses what it claims to
run: nix shell nixpkgs#trealla --command ./scripts/checkers.sh kb
# Every suite goes through scripts/checkers.sh, which pins a bash >= 5
# and scrubs the environment, so the shell and the variables a suite
# passes under here are the ones it passes under locally (improvise-f9qf).
run: ./scripts/checkers.sh kb
# Every suite goes through scripts/checkers.sh, which scrubs the
# environment, so the variables a suite passes under here are the ones it
# passes under locally (improvise-f9qf). Its bash is the dev shell's.
- name: The checker runner refuses what it claims to
run: ./scripts/checkers.sh --self-test
# pr-watch is driven entirely from fixtures. The release preflight is the
@@ -301,20 +237,13 @@ jobs:
# failed the *Browser harness* job. `-p opfs-contract` scopes it to 1 file.
harness-lint:
name: Lint the harness crate
runs-on: ubuntu-latest
runs-on: [self-hosted, nixos, microvm]
steps:
- uses: actions/checkout@v7
- uses: dtolnay/rust-toolchain@stable
with:
targets: wasm32-unknown-unknown
components: clippy, rustfmt
- uses: Swatinem/rust-cache@v2
with:
workspaces: harness/opfs-contract -> target
- working-directory: harness/opfs-contract
run: |
cargo fmt -p opfs-contract -- --check
cargo clippy --target wasm32-unknown-unknown --all-targets -- -D warnings
# checks.harness-lint in nix/ci.nix, run from inside harness/opfs-contract:
# `cargo fmt -p opfs-contract -- --check` and clippy for wasm32.
- name: cargo fmt -p opfs-contract and clippy --target wasm32
run: nix build --print-build-logs .#checks.x86_64-linux.harness-lint
# ---------------------------------------------------------------------------
# The browser harness (improvise-vvl3, absorbing improvise-xf1).
@@ -335,37 +264,15 @@ jobs:
browser:
name: Browser harness (OPFS contract + shell)
needs: build
runs-on: ubuntu-latest
runs-on: [self-hosted, nixos, microvm]
steps:
- uses: actions/checkout@v7
- uses: actions/setup-node@v4
with:
node-version: 22
- name: Install the harness
working-directory: harness
run: npm ci
# Keyed on the resolved @playwright/test version, so a version bump is a
# miss and anything else is a hit. The download was 50s on run
# 35535225637 and is pure I/O — nothing about it depends on this commit.
- name: Playwright version
id: pw
working-directory: harness
run: echo "v=$(node -p "require('@playwright/test/package.json').version")" >> "$GITHUB_OUTPUT"
- uses: actions/cache@v4
id: pw-cache
with:
path: ~/.cache/ms-playwright
key: playwright-${{ runner.os }}-${{ steps.pw.outputs.v }}
# All three, because improvise-xf1's acceptance criterion names Chrome,
# Firefox and Safari and `createSyncAccessHandle` is implemented
# separately by each engine. WebKit is installed even though its OPFS is
# absent today: the spec skips on a runtime probe, so the day that build
# gains OPFS the check starts running without a CI change.
# `--with-deps` installs apt packages OUTSIDE the cached directory, so it
# runs on a cache hit too; only the browser download is skipped.
- name: Install browsers
working-directory: harness
run: npx playwright install --with-deps chromium firefox webkit
# No browser install step and no version probe: the dev shell sets
# PLAYWRIGHT_BROWSERS_PATH to nixpkgs' browsers, which pin them. `npm ci`
# stays because @playwright/test itself is not nix-provided.
# The tree `build` assembled, not one this job rebuilds. Same bytes as
# the Pages artifact, modulo the cache-bust token `web/assemble.sh`
# stamps — the harness tree is stamped `harness` and the site the commit
@@ -389,9 +296,13 @@ jobs:
# renamed export is cheaper to read here than as a blank page.
- name: The page's calls match the bundle it calls
run: ./scripts/node-test.sh 'web/tests/bundle/*.test.js'
# Chromium and Firefox only, for now. nixpkgs' WebKit (WPEWebProcess)
# aborts on its first page inside the runner VMs, though not on the
# host; its four specs skip on the OPFS probe anyway, so nothing gated
# is lost. Put `--project=webkit` back once that abort is understood.
- name: Run the harness
working-directory: harness
run: npx playwright test
run: npx playwright test --project=chromium --project=firefox
- if: failure()
uses: actions/upload-artifact@v7
with:
@@ -432,9 +343,9 @@ jobs:
# exists to stop.
pages-deploy:
name: Deploy to GitHub Pages
needs: [test, wasm, js, browser, harness-lint, kb, build]
needs: [test, wasm, js, browser, harness-lint, kb, build, nix-build]
if: github.event_name == 'push' && github.ref == 'refs/heads/main'
runs-on: ubuntu-latest
runs-on: [self-hosted, nixos, microvm]
permissions:
contents: read
pages: write
Cargo.nix
+97 -91
View File
@@ -2640,7 +2640,6 @@ rec {
{
name = "indexmap";
packageId = "indexmap";
features = [ "serde" ];
}
{
name = "ratatui";
@@ -2654,6 +2653,7 @@ rec {
{
name = "serde_json";
packageId = "serde_json";
features = [ "float_roundtrip" ];
}
{
name = "thiserror";
@@ -2673,6 +2673,10 @@ rec {
name = "improvise-wasm-client";
packageId = "improvise-wasm-client";
}
{
name = "libc";
packageId = "libc";
}
{
name = "pest";
packageId = "pest";
@@ -2735,6 +2739,7 @@ rec {
{
name = "serde_json";
packageId = "serde_json";
features = [ "float_roundtrip" ];
}
{
name = "unicode-width";
@@ -2776,7 +2781,6 @@ rec {
{
name = "indexmap";
packageId = "indexmap";
features = [ "serde" ];
}
{
name = "serde";
@@ -2898,7 +2902,6 @@ rec {
{
name = "indexmap";
packageId = "indexmap";
features = [ "serde" ];
}
{
name = "js-sys";
@@ -2921,6 +2924,7 @@ rec {
{
name = "serde_json";
packageId = "serde_json";
features = [ "float_roundtrip" ];
}
{
name = "wasm-bindgen";
@@ -2975,6 +2979,7 @@ rec {
name = "serde_json";
packageId = "serde_json";
optional = true;
features = [ "float_roundtrip" ];
}
{
name = "unicode-width";
@@ -3011,6 +3016,7 @@ rec {
{
name = "serde_json";
packageId = "serde_json";
features = [ "float_roundtrip" ];
}
{
name = "wasm-bindgen";
@@ -3068,6 +3074,7 @@ rec {
{
name = "serde_json";
packageId = "serde_json";
features = [ "float_roundtrip" ];
}
{
name = "wasm-bindgen";
@@ -3135,6 +3142,7 @@ rec {
{
name = "serde_json";
packageId = "serde_json";
features = [ "float_roundtrip" ];
}
{
name = "tokio";
@@ -3171,27 +3179,6 @@ rec {
packageId = "hashbrown 0.17.1";
usesDefaultFeatures = false;
}
{
name = "serde";
packageId = "serde";
optional = true;
usesDefaultFeatures = false;
target = { target, features }: false;
}
{
name = "serde_core";
packageId = "serde_core";
optional = true;
usesDefaultFeatures = false;
}
];
devDependencies = [
{
name = "serde";
packageId = "serde";
usesDefaultFeatures = false;
features = [ "derive" ];
}
];
features = {
"arbitrary" = [ "dep:arbitrary" ];
@@ -3202,7 +3189,7 @@ rec {
"serde" = [ "dep:serde_core" "dep:serde" ];
"sval" = [ "dep:sval" ];
};
resolvedDefaultFeatures = [ "default" "serde" "std" ];
resolvedDefaultFeatures = [ "default" "std" ];
};
"indoc" = rec {
crateName = "indoc";
@@ -5655,7 +5642,7 @@ rec {
"preserve_order" = [ "indexmap" "std" ];
"std" = [ "memchr/std" "serde_core/std" ];
};
resolvedDefaultFeatures = [ "default" "std" ];
resolvedDefaultFeatures = [ "default" "float_roundtrip" "std" ];
};
"sha1" = rec {
crateName = "sha1";
@@ -8871,7 +8858,15 @@ rec {
)
crateConfigs;
target = makeTarget pkgs.stdenv.hostPlatform;
build = mkBuiltByPackageIdByPkgs pkgs.buildPackages;
# Build-time dependency graph (for proc-macros and build
# dependencies). When not cross-compiling it equals the host
# graph, so reuse `self`; otherwise build it for
# `pkgs.buildPackages`.
build =
if pkgs.stdenv.buildPlatform.config == pkgs.stdenv.hostPlatform.config then
self
else
mkBuiltByPackageIdByPkgs pkgs.buildPackages;
};
in
self;
@@ -8887,38 +8882,35 @@ rec {
devDependencies = lib.optionals (runTests && packageId == rootPackageId) (
crateConfig'.devDependencies or [ ]
);
dependencies = dependencyDerivations {
# Enabled (platform- and feature-filtered) dependency lists, reused
# for both derivation wiring and the crate renames below.
enabledDependencies = filterEnabledDependencies {
inherit features;
inherit (self) target;
buildByPackageId =
depPackageId:
# proc_macro crates must be compiled for the build architecture
if crateConfigs.${depPackageId}.procMacro or false then
self.build.crates.${depPackageId}
else
self.crates.${depPackageId};
dependencies = (crateConfig.dependencies or [ ]) ++ devDependencies;
};
buildDependencies = dependencyDerivations {
enabledBuildDependencies = filterEnabledDependencies {
inherit features;
inherit (self.build) target;
buildByPackageId = depPackageId: self.build.crates.${depPackageId};
dependencies = crateConfig.buildDependencies or [ ];
};
dependencies = map
(
dependency:
# proc_macro crates must be compiled for the build architecture
if crateConfigs.${dependency.packageId}.procMacro or false then
self.build.crates.${dependency.packageId}
else
self.crates.${dependency.packageId}
)
enabledDependencies;
buildDependencies = map
(dependency: self.build.crates.${dependency.packageId})
enabledBuildDependencies;
# Order (build dependencies, then normal dependencies) feeds the
# crateRenames grouping below.
dependenciesWithRenames =
let
buildDeps = filterEnabledDependencies {
inherit features;
inherit (self) target;
dependencies = crateConfig.dependencies or [ ] ++ devDependencies;
};
hostDeps = filterEnabledDependencies {
inherit features;
inherit (self.build) target;
dependencies = crateConfig.buildDependencies or [ ];
};
in
lib.filter (d: d ? "rename") (hostDeps ++ buildDeps);
lib.filter (d: d ? "rename") (enabledBuildDependencies ++ enabledDependencies);
# Crate renames have the form:
#
# {
@@ -9090,6 +9082,17 @@ rec {
corresponding feature sets are merged. Features in rust are additive.
*/
mergePackageFeatures =
args: builtins.mapAttrs (_packageId: builtins.attrNames) (mergePackageFeaturesImpl args);
/*
Core of the feature-resolution fixpoint. The cache (`featuresByPackageId`)
maps each packageId to a feature *set* (an attrset `feature -> 1`) rather
than a sorted list, so the fold merges with `//` and detects convergence
with attrset equality instead of re-concatenating and re-sorting the
accumulated feature list on every step. `mergePackageFeatures` projects the
result back to canonical sorted lists.
*/
mergePackageFeaturesImpl =
{ crateConfigs ? crates
, packageId
, rootPackageId ? packageId
@@ -9138,14 +9141,15 @@ rec {
cache:
{ packageId, features }:
let
cacheFeatures = cache.${packageId} or [ ];
combinedFeatures = sortedUnique (cacheFeatures ++ features);
cacheFeatures = cache.${packageId} or { };
# `features` is the (small) incoming list; merge it into the set.
combinedFeatures = cacheFeatures // listToSet features;
in
if cache ? ${packageId} && cache.${packageId} == combinedFeatures then
if cache ? ${packageId} && cacheFeatures == combinedFeatures then
cache
else
mergePackageFeatures {
features = combinedFeatures;
mergePackageFeaturesImpl {
features = builtins.attrNames combinedFeatures;
featuresByPackageId = cache;
inherit
crateConfigs
@@ -9158,8 +9162,8 @@ rec {
);
cacheWithSelf =
let
cacheFeatures = featuresByPackageId.${packageId} or [ ];
combinedFeatures = sortedUnique (cacheFeatures ++ enabledFeatures);
cacheFeatures = featuresByPackageId.${packageId} or { };
combinedFeatures = cacheFeatures // listToSet enabledFeatures;
in
featuresByPackageId
// {
@@ -9186,27 +9190,31 @@ rec {
assert (builtins.isList features);
assert (builtins.isAttrs target);
let
# Identical for every dep in this call; build the predicate arg once.
targetArgs = { inherit features target; };
in
lib.filter
(
dep:
let
targetFunc = dep.target or (features: true);
in
targetFunc { inherit features target; }
(dep.target or (features: true)) targetArgs
&& (!(dep.optional or false) || builtins.any (doesFeatureEnableDependency dep) features)
)
dependencies;
# Returns whether the given feature should enable the given dependency.
doesFeatureEnableDependency =
dependency: feature:
dependency:
# Callers partially apply this once per dependency, then test every feature,
# so hoist the dep-invariant strings out of the per-feature comparison.
let
name = dependency.rename or dependency.name;
depName = "dep:" + name;
prefix = "${name}/";
len = builtins.stringLength prefix;
startsWithPrefix = builtins.substring 0 len feature == prefix;
in
feature == name || feature == "dep:" + name || startsWithPrefix;
feature:
feature == name || feature == depName || builtins.substring 0 len feature == prefix;
/*
Returns the expanded features for the given inputFeatures by applying the
@@ -9220,29 +9228,24 @@ rec {
assert (builtins.isAttrs featureMap);
assert (builtins.isList inputFeatures);
let
# Transitive closure of `inputFeatures` under `featureMap`. `seen`
# tracks already-expanded features so each is visited at most once;
# this also breaks feature cycles (issue #209).
expandFeaturesNoCycle =
oldSeen: inputFeatures:
if inputFeatures != [ ] then
let
# The feature we're currently expanding.
feature = builtins.head inputFeatures;
# All the features we've seen/expanded so far, including the one
# we're currently processing.
seen = oldSeen // {
${feature} = 1;
};
# Expand the feature but be careful to not re-introduce a feature
# that we've already seen: this can easily cause a cycle, see issue
# #209.
enables = builtins.filter (f: !(seen ? "${f}")) (featureMap."${feature}" or [ ]);
in
[ feature ] ++ (expandFeaturesNoCycle seen (builtins.tail inputFeatures ++ enables))
# No more features left, nothing to expand to.
else
[ ];
outFeatures = expandFeaturesNoCycle { } inputFeatures;
seen: features:
builtins.foldl'
(
acc: feature:
if acc ? ${feature} then
acc
else
expandFeaturesNoCycle (acc // { ${feature} = 1; }) (featureMap.${feature} or [ ])
)
seen
features;
seen = expandFeaturesNoCycle { } inputFeatures;
in
sortedUnique outFeatures;
sortedUnique (builtins.attrNames seen);
/*
This function adds optional dependencies as features if they are enabled
@@ -9301,16 +9304,19 @@ rec {
in
defaultOrNil ++ explicitFeatures ++ additionalDependencyFeatures;
# Builds a feature set (attrset `feature -> 1`) from a list of feature names.
listToSet =
features:
builtins.listToAttrs (map (feature: { name = feature; value = 1; }) features);
# Sorts and removes duplicates from a list of strings.
sortedUnique =
features:
assert (builtins.isList features);
assert (builtins.all builtins.isString features);
let
outFeaturesSet = lib.foldl (set: feature: set // { "${feature}" = 1; }) { } features;
outFeaturesUnique = builtins.attrNames outFeaturesSet;
in
builtins.sort (a: b: a < b) outFeaturesUnique;
# attrNames returns keys in ascending string order, so the result is
# sorted and deduplicated. The feature-merge fixpoint relies on this
# canonical order to detect convergence.
builtins.attrNames (listToSet features);
deprecationWarning =
message: value:
benches/grid_projection.rs
+114 -2
View File
@@ -54,6 +54,102 @@ fn dense_app(regions: usize, products: usize, measures: usize) -> App {
app
}
/// Collapsed groups on the row axis and a formula chain on the column axis, so
/// every fold cell and every total re-derives the chain (improvise-h6z1).
fn chain_app(regions: usize, products: usize) -> App {
let mut wb = Workbook::new("Chain");
wb.add_category("Region").unwrap();
wb.add_category("Product").unwrap();
for r in 0..regions {
wb.add_item_in_group("Region", format!("Region{r}"), format!("G{}", r / 10))
.unwrap();
}
for p in 0..products {
wb.add_item("Product", format!("Product{p}")).unwrap();
}
wb.add_item("_Measure", "Revenue").unwrap();
let chain = [
"F1 = Revenue * 2",
"F2 = F1 + Revenue",
"F3 = F2 + F1",
"F4 = F3 + F2",
"F5 = F4 / F1",
];
for text in chain {
let formula = improvise::formula::parse_formula(text, "_Measure").unwrap();
wb.model.add_formula(formula);
}
{
let v = wb.active_view_mut();
v.set_axis("Region", Axis::Row);
v.set_axis("Product", Axis::Row);
v.set_axis("_Measure", Axis::Column);
for g in 0..regions.div_ceil(10) {
v.toggle_group_collapse("Region", &format!("G{g}"));
}
}
let mut n = 0.0f64;
for r in 0..regions {
for p in 0..products {
n += 1.0;
let key = CellKey::new(vec![
("Region".to_string(), format!("Region{r}")),
("Product".to_string(), format!("Product{p}")),
("_Measure".to_string(), "Revenue".to_string()),
]);
wb.model.set_cell(key, CellValue::Number(n));
}
}
let mut app = App::new(wb, None);
app.rebuild_layout();
app
}
/// Days in collapsed groups of `per_group` on Row, with Product: every row is a
/// fold over a few days, never the whole category (improvise-jq0m).
fn grouped_app(days: usize, per_group: usize, products: usize, measures: usize) -> App {
let mut wb = Workbook::new("Grouped");
wb.add_category("Day").unwrap();
wb.add_category("Product").unwrap();
for d in 0..days {
let group = format!("G{}", d / per_group);
wb.add_item_in_group("Day", format!("Day{d}"), group)
.unwrap();
}
for p in 0..products {
wb.add_item("Product", format!("Product{p}")).unwrap();
}
for m in 0..measures {
wb.add_item("_Measure", format!("Measure{m}")).unwrap();
}
{
let v = wb.active_view_mut();
v.set_axis("Day", Axis::Row);
v.set_axis("Product", Axis::Row);
v.set_axis("_Measure", Axis::Column);
for g in 0..days.div_ceil(per_group) {
v.toggle_group_collapse("Day", &format!("G{g}"));
}
}
let mut n = 0.0f64;
for d in 0..days {
for p in 0..products {
for m in 0..measures {
n += 1.0;
let key = CellKey::new(vec![
("Day".to_string(), format!("Day{d}")),
("Product".to_string(), format!("Product{p}")),
("_Measure".to_string(), format!("Measure{m}")),
]);
wb.model.set_cell(key, CellValue::Number(n));
}
}
}
let mut app = App::new(wb, None);
app.rebuild_layout();
app
}
fn whole_grid(app: &App) -> Viewport {
Viewport {
row_offset: 0,
@@ -105,7 +201,10 @@ fn time_widths(iters: u32, app: &App) -> Duration {
}
fn case(label: &str, regions: usize, products: usize, measures: usize, iters: u32) {
let app = dense_app(regions, products, measures);
report(label, dense_app(regions, products, measures), iters);
}
fn report(label: &str, app: App, iters: u32) {
let rows = app.layout.row_count();
let cols = app.layout.col_count();
let cells = rows * cols;
@@ -120,9 +219,12 @@ fn case(label: &str, regions: usize, products: usize, measures: usize, iters: u3
true => "UNSTABLE".to_string(),
false => format!("{share:.0}%"),
};
// The TUI pays both on every keypress: widths in `cmd_context`, the grid in draw.
let keypress = full + widths;
println!(
"{label:<22} {rows:>5} x {cols:>3} = {cells:>7} cells whole-grid {full:>11.3?} \
windowed {windowed:>11.3?} ratio {ratio:>4.1}x col_widths {widths:>11.3?} ({share} of the frame)"
windowed {windowed:>11.3?} ratio {ratio:>4.1}x col_widths {widths:>11.3?} ({share} of the frame) \
keypress {keypress:>11.3?}"
);
}
@@ -130,7 +232,17 @@ fn main() {
println!("project::grid_of, one call = one TUI frame's grid cache");
println!("a 60fps frame budget is 16.7ms; the TUI redraws on each keypress\n");
case("typical", 20, 5, 4, 200);
case("1.2k cells", 60, 5, 4, 100);
case("2.4k cells", 60, 10, 4, 100);
case("4.8k cells", 60, 20, 4, 50);
case("large", 60, 20, 8, 50);
case("18k cells (gotcha 41)", 150, 20, 6, 20);
case("pathological", 300, 30, 10, 5);
println!("\nfolded rows and a five-formula chain (improvise-h6z1)\n");
report("folded chain ~18k", chain_app(1000, 30), 5);
report("folded chain ~90k", chain_app(5000, 30), 3);
println!("\ncollapsed small groups on the row axis (improvise-jq0m)\n");
report("364 days in weeks", grouped_app(364, 7, 20, 6), 5);
report("364 days in pairs", grouped_app(364, 2, 20, 6), 5);
report("1000 days in pairs", grouped_app(1000, 2, 10, 4), 3);
}
context/code/clients.md
+50 -3
View File
@@ -516,6 +516,21 @@ seeds the `edit` buffer from the cell it landed on. The trailing `clear-buffer`
the keymap welds to every commit then empties that buffer again. That ordering
looks wrong and is exactly what the TUI does; mirroring it is the point.
The re-entry is `reopened_at`, and it declines where the host's
`EnterEditAtCursor` declines (improvise-enz6). The host asks
`core::uneditable` of its layout. The browser asks the landing cell's projected
`CellId`, which is the same answer, because `cell_id` is what fills it. Two
limits follow from asking the cache:
- **The wording differs on a fold.** The host can tell a fold from any other
aggregate and names the fold. The browser sees only `CellId::Aggregate`, so it
gives the host's generic aggregate text. `lockstep.rs` compares the mode, the
cursor and the buffers here, never the status text.
- **A landing past the window opens anyway.** The cache holds only the
announced window, so a `Down` from the window's last row lands on a cell with
no projected id. The browser then opens the editor, where the host does not.
No write can follow it: once the host has projected that cell,
`record_at_cursor` finds no record there and the commit composes nothing.
`GridMove` is a hand-spelled mirror of `navigation.rs`'s `MoveKind`, because the
command crate is not on a browser's dependency list and never will be. The
*keys* that reach it are no longer spelled twice. `scrolled_to` mirrors
@@ -578,11 +593,43 @@ immutable `CmdContext`; the commands are then folded in order.
### What still needs the host
`the_command_names_that_still_need_the_host_are_these` names them, and
`Host::handle` answers a `Key` frame with an empty vec, so each one is a binding
that **does nothing at all** in a browser
`resolve::NEEDS_THE_HOST` names them, and
`the_command_names_that_still_need_the_host_are_these` drives each through the
key that reaches it. `Host::handle` answers a `Key` frame with an empty vec, so
each one is a binding that **does nothing at all** in a browser
(`doctrine(an_upstream_key_is_a_black_hole)`).
**The help overlay marks them, and it marks rather than omits** (improvise-uln5).
The overlay is shared content, and before this the browser painted every row
plainly, including six that do nothing there. That is worse than a gap:
improvise-koh6 showed that the keys typed after a dead one land on live Normal
bindings, and `i` opens an editor. The browser's `RenderHelpRow` now draws a row
whose command is on the list as `dim`, and appends "(not in the browser yet)"
to its description. It does the same for every ex-command row while `:` opens a
command line this client cannot run (`resolve::runs_the_command_line`, which is
`stranding`'s answer, not a second one).
Marking was chosen over omitting for three reasons:
- **Both front ends draw the same rows.** A page with rows missing is a second
layout of the same content.
- **The feature stays findable.** The row still says it exists in the terminal.
- **It needs no change in `web/`.** `dim` is a kind the page already paints.
The TUI is unaffected, because the marking lives in the browser's renderer.
**One list, held from both sides.** `NEEDS_THE_HOST` is production data because
the renderer needs it. Two tests hold it to what `resolve` actually does:
- `the_command_names_that_still_need_the_host_are_these` fails if a name on it
resolves locally.
- `every_help_row_the_browser_cannot_run_is_one_it_marks` fails if a help row
is silent in the browser without being on the list, or on the list without
being silent. It also fails if a *hint line* ever names a listed command,
because the status bar is never marked.
`every_advertised_command_answers_in_its_own_mode` therefore no longer restates
the black holes. It skips a silent row whose command is on the list, because
the other test owns it.
**An upstream key that the app also ADVERTISES is a different severity, and it
had its own class of defect** (improvise-rmvx). A key nobody offers doing
nothing is a gap; a key named in the mode's own hint line doing nothing reads as
context/code/command.md
+225 -13
View File
@@ -609,6 +609,47 @@ the client was told nothing at all.
`AddItem` case the same way and found none, so `AddItem` does not stop either.
An abort is added where a later effect depends on the failed one, never for
symmetry.
- **The `|| last_batch_aborted()` arm is kept, and the invariant it guards is
pinned (improvise-we1a).** Every model effect that aborts today also refuses,
so deleting the arm left the workspace green: nothing tested it. There were
two ways to close that. One was to make an abort without a refusal
impossible to write. The other, taken here, was to test the invariant with
the arm left in place. `every_failing_model_effect_that_aborts_also_refuses`
applies each failing `ModelCommand`'s own effects to an `App`, bypassing
`Host`, and asserts it refused. It also pins which cases abort, so it cannot
pass on a table that aborts nothing. The wire table cannot hold this
invariant, because the arm itself rejects a frame that aborted without
refusing. With AddCategory's arm reverted to status-only, the wire table
stays green. The option not taken is right for today's rules and wrong for
the future:
- `AbortChain` aborts without refusing, correctly. It is `ViewOnly`, and it
stops a chain whose premise no longer holds, which is not an error. A
type-level rule of "every abort refuses" would first have to exempt it.
- Making it refuse would change which `:` lines the headless CLI reports as
failures.
- Keeping the raw `abort_effects` setter away from `Model` effects alone would
be a new visibility split, and today no effect needs it.
So the arm stays, as defence for a future `Model` effect that aborts on
something other than an error.
**The table alone did not hold the invariant (improvise-lasd).** It lists
failing commands, so an abort at a site it does not list got past it. With
`on_view`'s missing-view branch changed to abort without refusing, the
workspace stayed green, and only the arm caught it. So `App::apply_effects`
now carries the rule itself. When the abort flag is set after a `Model`
effect, a `debug_assert!` requires that the batch has refused. `ViewOnly`
effects are exempt, so `AbortChain` is untouched. That answers the objection
above: the rule is on `EffectKind`, not a new visibility split.
- **Coverage:** it covers every model effect that any test drives to an
abort, listed or not. Under the `on_view` rival it fails two existing
tests.
- **Where it stops:** it is a debug assertion, so release builds rely on the
arm. It reads the batch-wide `batch_refused`, so a model effect that aborts
without refusing after an earlier effect in the same batch has refused
still gets past it.
- **Why the table stays:** it drives every failing command, and so it
exercises the assertion.
In a client's frame, any refusal becomes `Rejected(MutationRefused)`. The frame
is undone whole, and the client is told, which is the channel the narrated
@@ -637,12 +678,13 @@ Consequences:
suffices. So each `refuse_and_stop` site needs a key-path assertion for its
refusal as well as one for its abort. The abort is
`a_failed_load_or_import_does_not_run_the_quit_after_it` (improvise-514o).
Three sites are not in the key-path table (improvise-rtud). The quit test's
"setup:" assertion holds the refusal of `load` and `import-json absent.json`.
Every `refuse_and_stop` site now has a row in
`every_refusal_site_refuses_its_failure_and_only_its_failure`. The last three
were added by improvise-we1a: `load`, `import-json absent.json`, and
`AddFormula`'s parse arm. Before that, only indirect assertions held them.
The quit test's "setup:" assertion held `load` and `import-json absent.json`.
`a_formula_that_does_not_parse_keeps_the_user_in_the_editor_with_their_text`
holds `AddFormula`'s parse arm, which has no case in the quit test. The other
five import arms are in
`every_refusal_site_refuses_its_failure_and_only_its_failure`.
held the parse arm (improvise-rtud). Those assertions remain.
Each case in that table has a succeeding twin, so a site that refused
unconditionally would fail too. That matters because these are exactly the
lines a merge conflict could silently drop.
@@ -678,6 +720,15 @@ The effect itself never inspects `app.view_state.mode`: the mode is decided
statically by whoever invoked it. An earlier version branched on
`app.view_state.mode.is_records()` and got it wrong mid-batch.
It opens nothing on a cell `i` declines, and says why (improvise-enz6). The
question is `cmd::core::uneditable`, the one `committable_cell_key` asks. A
fold or another aggregate, or a formula's result, gets its refusal text in
the status line. The mode stays where the batch put it. `CommitAndAdvance`
already dropped to normal at its head (gotcha 21), so the user lands on the
fold in normal mode. It is a status line and not `App::refuse`, because the
commit before it succeeded. A `commit-cell-edit` script line whose advance lands on a
fold must not report its write as refused.
### Which effects reset the cursor, and why it is decided one at a time
Swapping the axes renumbers every row and column, so the old indices name
@@ -1753,6 +1804,140 @@ frames at 60fps, and the remaining cost is one genuine evaluation per cell. The
next lever is caching *across* frames, which needs an invalidation story the
`record_versions`/`layout_version` tiers could plausibly supply.
**The totals left the walk (improvise-wm20).** A column's total used to be the
walk's running sum of its cells, which is wrong for everything except a stored
measure under Sum or Count. It gave a ratio column the sum of its rows' ratios,
an Avg measure the sum of its rows' means, and a text column `0.00`. The user's
rule is that **a column's total is its cell as if every drawn row were one
fold**. `GridLayout::column_total` asks `Model::evaluate_spanning` for the
column key with the row categories spanned, so a formula is evaluated over the
pooled rows (Margin = total Profit / total Revenue) and a stored measure folds
its records under its own aggregator. The user ruled on three cases:
- **Text** follows the unanimity rule (gotcha 7): the text if every row agrees,
otherwise the `mixed` error.
- **An error** in any row makes the total an error, which is what that rule
answers for an error among numbers. That holds for a **stored** error. A
formula column is evaluated over the pooled inputs, so a row's `div/0` is not
an input and does not propagate. Only the fold's own error does. Rulings (1)
and (2) disagree here: the column shows an error in one row and a clean
number under it. The user chose the **pooled ratio** (improvise-b5dp). Margin
over rows 25/30, 27/30 and -4/0 totals 48/60 = 0.80, not `div/0`.
- **With `_Measure` on the row axis** the strip is empty, because a column adds
different measures together. An empty strip already means "no totals"
(gotcha 33).
The coverage is the **drawn** rows, as a fold's is (improvise-f2oo): a hidden
row stays out, and so does a record that lacks a row coordinate. That
deliberately differs from `Axis::None`, which counts both. The browser and the
TUI agree by construction, since both draw the projected `GridCache.totals`.
The cost is one more evaluation per column, and it is not free. The same
`cargo bench --bench grid_projection` on the orchestrator's Mac (Apple M4 Max),
before and after:
| case | whole grid before | after | `col_widths` before | after |
|---|---|---|---|---|
| typical (400) | 0.86 ms | 1.24 ms | 0.53 ms | 0.92 ms |
| 18k | 114 ms | 177 ms | 75 ms | 142 ms |
| pathological (90,000) | 1.54 s | 3.49 s | 1.07 s | 1.96 s |
`col_widths` is the keypress path, and it pays too, because a total's text sizes
its column. The event loop draws once per key, so every key pays the fold twice:
once in `App::cmd_context` for `visible_cols`, and once in the draw's `grid_of`.
At 4.8k cells that took a held arrow key from about 31 ms to about 52 ms, past
the ~33 ms key-repeat interval (improvise-yb4b). The walk no longer carries a
sum, so the window-into-totals fault below cannot happen any more. The test
still guards the widths half.
**The fold scans instead of enumerating (improvise-yb4b).** The cost was not the
fold's arithmetic. It was *how the fold found its records*: `fold_stored` built
one `CellKey` per member, the cross product of every spanned category's covered
items (3,000 keys for an 18k column), and looked each one up. Now
`group_expansion` returns the expansion as data: a `base` key and the covered
items of each expanded category. `fold_stored` hands that to
`DataStore::spanned_values`, which narrows through the index and filters each
candidate by membership. In `Exact` mode a candidate must also carry no
coordinate beyond those, which is what `get(member)` answered. Only a formula
still enumerates members (`contexts_of`), since its references are evaluated
per context.
**How it narrows is a choice, and the first version got it wrong
(improvise-jq0m).** Narrowing by `base` alone is right for a column total, which
spans *every* item of each row category. It is wrong for a collapsed group,
which spans two or seven items of a category with hundreds: `base` drops the
grouped category entirely, so every fold cell scanned every record of the
column, and a grid of small groups went quadratic. The review measured 1000
days in pairs at 11–15.7 s a keypress, against 0.44–0.51 s before #239. So
`spanned_positions` now prices both narrowings off the index's own set sizes
and takes the cheaper: once by `base`, or once per covered item of the span
with the fewest items, each intersected with `base`. The per-item positions are
disjoint (a record has one item per category) and are sorted afterwards, so
both paths return the store's order. A `base` naming a name the store never
interned matches nothing on both paths.
The bench now carries the grouped cases, which it could not see before: Day in
collapsed groups on Row, with Product, and `_Measure` on Column. The machine is
an Apple M4 Max (16 cores, 64 GB) at load average 15–63. The figures are
keypress (`whole-grid + col_widths`), min-of-N, from two alternating rounds,
the second reversed. Each cell gives both rounds:
| case | before #239 (`5932ad74`) | `f668ddc9` | jq0m | jq0m ÷ before |
|---|---|---|---|---|
| 4.8k dense | 33.9 / 39.1 ms | 40.7 / 40.2 ms | 36.3 / 33.3 ms | 1.07 / 0.85 |
| 18k dense | 237 / 239 ms | 238 / 232 ms | 196 / 211 ms | 0.83 / 0.88 |
| 90k dense | 3.04 / 3.46 s | 3.24 / 2.70 s | 2.68 / 2.84 s | 0.88 / 0.82 |
| 364 days in weeks | 483 / 415 ms | 1.42 / 1.18 s | 356 / 399 ms | 0.74 / 0.96 |
| 364 days in pairs | 715 / 539 ms | 5.02 / 4.62 s | 471 / 518 ms | 0.66 / 0.96 |
| 1000 days in pairs | 406 / 399 ms | 10.8 / 20.2 s | 352 / 364 ms | 0.87 / 0.91 |
The grouped cases now come in **below** the pre-#239 figures. The old
per-member path looked up each member through a full `CellKey`, and the
per-item narrowing intersects index sets directly.
Two levers were **not** taken. Combining the walk's row values for decomposable
aggregators was rejected under §5 as a second derivation of the total. It is
also wrong wherever a formula owns a row-axis item: the row reads the formula,
while the spanning fold reads the store. Sharing one total between
`cmd_context` and the draw was also rejected: a key's effects run between the
two, so sharing needs an invalidation story. Dropping the totals from the widths
would make the two width computations disagree about `visible_cols`.
**A grid-wide fold memo (improvise-h6z1)** is the other half, and it answers a
different cost. It removes the repeated **formula** chains that a fold row and a
total re-derive, about 2x on a folded five-formula chain. It does **not** move
the table below. Those grids hold only stored measures, a stored fold is never
memoised, and each total reads its column's records once. That one read per
column is what the scan above made cheap.
**What it changes besides speed:** a stored fold's values now arrive in the
store's insertion order, not member order. That is the order gotcha 18 names as
the store's answer. Min, Max, Count and text unanimity are order-free. A Sum or
Avg over non-integral values may differ in the last bits.
Measured by alternating three builds of `cargo bench --bench grid_projection` on
an Apple M4 Max (16 cores, 64 GB) under heavy shared load (load average 30–90).
This is min-of-N, three rounds, with the order reversed in the third. The
keypress figure is `whole-grid + col_widths`. The millisecond columns are
ranges over the rounds. The ratio columns are each round's own pair, in round
order:
| cells | before #239 (`5932ad74`) | #239 (`9948aaf5`) | yb4b | #239 ÷ before | yb4b ÷ before |
|---|---|---|---|---|---|
| 400 | 1.36–1.77 ms | 2.36–2.76 ms | 1.69–1.82 ms | 2.03, 1.34, 1.78 | 1.24, 0.98, 1.25 |
| 1.2k | 4.1–5.2 ms | 7.2–8.2 ms | 5.0–5.3 ms | 2.00, 1.39, 1.85 | 1.23, 0.96, 1.25 |
| 2.4k | 10.8–13.6 ms | 18.4–21.5 ms | 12.0–13.9 ms | 1.98, 1.36, 1.91 | 1.18, 0.88, 1.29 |
| 4.8k | 32.8–41.8 ms | 55.4–64.1 ms | 34.5–42.0 ms | 1.77, 1.32, 1.92 | 1.15, 0.82, 1.26 |
| 9.6k | 102.8–121.2 ms | 190.7–209.6 ms | 107.2–140.0 ms | 2.04, 1.57, 1.92 | 1.36, 0.88, 1.20 |
| 18k | 192.9–208.3 ms | 338.6–448.0 ms | 230.4–273.3 ms | 2.32, 1.63, 1.72 | 1.42, 1.11, 1.23 |
| 90k | 2.68–3.26 s | 4.92–6.49 s | 3.18–4.00 s | 1.99, 1.84, 1.79 | 1.23, 1.47, 1.14 |
Read the ratio columns, not the milliseconds. Round 2's `before` ran at load
86 against about 31 for its `yb4b`, which is why its ratios fall below 1. The
two clean rounds put #239 at about 1.8–2.3× the pre-#239 keypress, and put
yb4b at about 1.15–1.4×. At 4.8k a held key is back under the ~33 ms repeat
interval in the quietest round (34.5 ms against 32.8 ms before).
The guard is `a_window_narrows_the_cells_and_neither_the_totals_nor_the_widths`:
with one walk serving three consumers, a window that leaked into the other two
would silently make a column total the sum of the visible rows, or size a column
@@ -3417,12 +3602,17 @@ the one writer that must see the synthetic key: renaming a coordinate column,
creating a column and staging a drill edit all resolve the records position
inside `commit_cell_value`. Its door refuses the two arms `i` refuses, an
aggregate (a fold included) and a formula's result. It needed one because
`Enter` and `Tab` advance the editor, and `EnterEditAtCursor` re-opens it at the
`Enter` and `Tab` advance the editor, and `EnterEditAtCursor` re-opened it at the
new cursor with no guard. `i` declines to edit `Profit`, yet `Tab` from `Cost`
landed the editor on it, and the next commit stored a value under the formula.
That is a fourth door nobody had listed. **The entry half is still open**
(improvise-enz6): the editor still opens on a fold or a formula's cell. Only the
write is refused, out loud, at commit. Closing the entry belongs in `ui/effect.rs`.
That is a fourth door nobody had listed. **The entry half is closed too**
(improvise-enz6). The door's two arms are now one function,
`core::uneditable`, and `EnterEditAtCursor` asks it before opening. So the
re-opened editor and the commit that would follow it cannot disagree about which
cells they decline. The commit door stays: a `:commit-cell-edit` line runs it at
the cursor with no editor opened there. Since the advance no longer reaches it,
the door has tests of its own that seat the editor directly
(`an_editor_already_open_on_a_fold_commits_nothing_there` and its formula twin).
**Drill keeps the narrowest door** because drilling into an aggregate is its
purpose. Only a fold is refused, since no record carries a group's name. The
@@ -3607,10 +3797,32 @@ the refusal marks the batch, `Host::mutating_frame` undoes the frame, the asker
gets `Rejected(MutationRefused)`, and the reason is on the host's status line.
The question it asks is the projection's own, never a second derivation of it:
- A coordinate that names a **group** rather than an item is refused, in any
view (improvise-b114). A group has no cell of its own, so writing one stores a
non-item coordinate. This rule asks the document, not the layout, and runs
first, because a fold in the measure half hides from both rules below. Take
- A coordinate that names **no item of its category** is refused, in any view
(improvise-b114, then improvise-nacd). This rule asks the document, not the
layout, and runs first, because a fold in the measure half hides from both
rules below.
**It is stated positively, and that is the point of the nacd change.** The
b114 rule was negative: refuse a name that *is a group*. The invariant it
protects (improvise-gw18) is positive: every coordinate names an item. A
negative rule covers only the ways of not being an item that someone listed,
and review found two more within a day. `Bogus` is no item and no group. A
parent group such as `Totals`, above `Money`, holds no item directly, so
`items_in_group("Totals")` is empty and the group test missed it. Both wrote
`{Month=Feb, _Measure=…}` onto a stored record and were accepted. The
positive rule covers both, and any shape not yet thought of, because it asks
whether the name *is* an item rather than what else it might be.
"Item" means `Model::effective_item_names`: the category's items, plus the
formula targets for `_Measure`. The formula targets are included so that a
write under a formula passes this rule and is refused by the next one with
the reason that is actually true ("computed by a formula"), not as a non-item.
The same-name case falls out: an item called `Money` beside a group called
`Money` is an item, and is writable (improvise-pr8t). The b114 rule needed an
explicit exemption for that case, and nothing tested the exemption.
A group has no cell of its own, so writing one stores a non-item
coordinate. Take
`{Month=Feb, _Measure=Money}`, where `Money` groups Revenue and Cost. Its
record `{Month=Feb}` is stored, so the second rule below accepts it and never
asks the projection. And with `Money` expanded the host draws no such cell at
context/code/core.md
+21 -8
View File
@@ -929,16 +929,29 @@ every member. Five decisions:
without the memo, about 60 ms with it, and about 10 ms on gh/main. gh/main
was cheap because it read the members out of the formula cache, which is
what gave the wrong ratios. The remaining ~6× is each column's cell
re-deriving the chain below it (45 evaluations per row where 9 would do). A
memo scoped to the whole grid would remove it; it is not built.
re-deriving the chain below it (45 evaluations per row where 9 would do).
**The memo is now shared across one grid build** (improvise-h6z1).
`viewport::grid_body` makes one `FoldMemo` and passes it through
`GridLayout::evaluate_in` and `column_total` for every cell and total, so a
fold row's chain is evaluated 9 times, not 45. A memo is valid only for the
model state it read. That is why it is made per build and never stored on
`GridLayout`, which outlives cell writes.
The memo is keyed by the fold key **and** the span's categories. Within one
grid build the span half cannot yet differ for one key, because a cell's key
always names the row categories and a total's never does. It is kept because
`evaluate_spanning_in` is public, and a caller sharing one memo across two
spans would otherwise read the wrong fold. No test can see it today.
- **A fold covers the members the grid draws** (improvise-f2oo). Hidden members
are not in it. So a fold is exactly the rows it replaces, the total of a
**stored** measure under Sum or Count does not move on `z`, and a group whose
every member is hidden has no row. Under Avg, Min or Max a stored measure's
total does move, and so does a **formula** column's (PR #203's review measured
Margin 1.90 expanded, 1.30 collapsed). The strip sums the rows' values rather
than folding the column the way a fold does, so it sums a Max row's maximum
and a formula row's result. That is improvise-wm20, and it is not this rule. That last
column's total does not move on `z`, and a group whose every member is hidden
has no row. That holds for every aggregator and for a formula column only
since improvise-wm20, because the strip used to **sum** the drawn rows. That
moved a Max measure's total on `z`, and a ratio's (PR #203's review measured
Margin 1.90 expanded and 1.30 collapsed). A total is now the column's cell as
if every drawn row were one fold, so collapsing a group changes nothing it
reads. The fold's machinery is shared through `Model::evaluate_spanning`: its
`Span` adds, to the groups a key names, the categories a key **omits**, which
it expands to their covered items. That last
point was already true, and it contradicted the old rule, under which a hidden
value appeared only while a sibling was visible. This deliberately **differs
from `Axis::None`**, where hidden items still count, because the two replace
context/code/protocol.md
+101 -1
View File
@@ -1997,7 +1997,107 @@ edit are the same string **by construction** rather than by agreement
(improvise-k8h) — the keymap never spells a buffer name.
`commit(key, cmd, args)` welds the command to its `clear-buffer` so no caller
can bind a commit without its clear (improvise-w3q). `rebind` exists for
can bind a commit without its clear (improvise-w3q).
**The two cell-editing commit keys are the exception, and the clear moved rather
than disappeared** (improvise-fy9o). `commit-cell-edit` and
`commit-and-advance-right` reopen an editor on the cell they advance to, and the
welded clear ran **after** them, so it blanked the re-seeded buffer. The
reopened editor looked empty, and a second Enter committed `""` and cleared the
landed cell. The user's ruling is that the reopened editor shows the landed
cell's current value, so with improvise-ixxx's untouched rule an unchanged Enter
writes nothing. `commit_reopening(key, cmd, args)` therefore binds the command
alone, and the command clears its own buffer **at its head**:
- in the TUI, `CommitAndAdvance` does it right after the mode change;
- in the browser, `resolve::cleared` does it, reached from both `commit` and
`create_column`.
Being at the head is also gotcha 21's rule: a head clear survives an abort
where the trailing weld did not. The corner commit and the `+cat` insertion
reopen nothing, and they still leave no text behind.
**After a write, the browser's reopened editor takes its seed from the host's
answer, not from its cache** (improvise-3uzg). The first fix (improvise-3tt4)
seeded from the cache everywhere except the cell the frame had just written.
That assumed the write changed the meaning of only that one cell, which is
false. Under `prune_empty`, emptying the last value in a row takes the row
away. The advance then lands on a renumbered position, and the cache still
holds the old record there. The browser opened on South's `2` while the TUI
opened on West's `3`, and typing `5` wrote 25 into West. Any write can change
what another position means, and the client cannot tell which. So the rule is
stated once, over the write, never over a list of cases:
- **A commit that writes** reopens its editor as `OpenedEditor::awaiting()`, an
empty buffer whose seed is still to come. `create_column` is included,
because `AddItem` adds a column.
- **`ClientState::settle_awaited_seed`** fills the seed from the grid the
client holds, but only once no frame it sent is still unanswered. The answer
folds the host's projection before `Accepted`, so the grid is the host's by
then. `accepted()` therefore settles.
- **A commit that writes nothing** awaits as well while any earlier frame's
write is unanswered. Otherwise it seeds from the cache at once. "Nothing
changed, so the cache is current" was the rule here, and it was false
(improvise-mdfc). *This* commit changed nothing, but an earlier one might
have, and the cache predates it until the answer arrives. Tab Tab pressed
quickly at the rightmost column opened the second editor on the value from
before the first Tab's write: 505 where the TUI stores 5095. So the test is
"does this frame write, or is a write still in flight", which is
`writes || in_flight.iter().any(|sent| sent.awaited)`. It checks *any*
in-flight write, not the newest frame: a frame that writes nothing stays in
the queue behind an unanswered write, so the newest frame can be one of
those.
- **Every opener asks the in-flight half, because it lives in `opened_on`**
(improvise-p6v6). mdfc first put the whole test inside `advanced`, which left
`i`/`a` (`edit_or_drill`) seeding from the cache with no in-flight check.
`i 9 Tab Esc i 5 Enter`, typed faster than the answer, stored 15 in the
browser where the TUI stores 195. The two halves now live in different
places, for a reason. "A write is still in flight" is true of the *client*,
so `opened_on` asks it, and no opener can forget. "This frame writes" is
true only of the commit being resolved, which is not in `in_flight` yet, so
`advanced` still asks it itself. Folding both into `opened_on` would lose the
second: the 3tt4 and 3uzg tests fail when `advanced` defers entirely.
The click-edit path (`socket.rs::edit_target_at`, improvise-ugj4) also seeds
from the cache, but it claims the record at the version it *read*. Behind an
unanswered write the host therefore refuses its commit as `RecordMoved`: a
loud failure, not a silent overwrite.
- **The seed settles only into an open editor** (improvise-4cb5). An Esc typed
before the answer arrives leaves the awaiting `opened_editor` in place. The
late seed would then fill the edit buffer in `Normal` mode, where the TUI's is
empty.
- **`awaiting` has no `serde(default)`** (improvise-mgis), for the reason
`AxisFrame::first` has none (§3a, improvise-1wl0). `awaiting: false` is a
claim, "the seed is settled". A peer that dropped the field would hand over an
editor that stays empty forever, and whose Enter writes nothing. A missing
field is therefore a parse failure.
Asking the host to send the seed was the alternative. It would have needed a
new message, and the host already sends it: the landed cell's `edit_text` in
the projection it owes this client anyway. Two things are accepted costs:
- Until the answer arrives, the editor is empty. Over a worker that takes
well under a frame.
- A key typed before the answer arrives marks the editor touched, and the late
seed then does not overwrite it. What the user sees is then what gets
written. That is never another record's number only because of the mdfc
rule above: before it, an untouched commit behind an unanswered write opened
on the stale cache, and typing then extended the wrong number. What is left
is a divergence from the TUI rather than a wrong record. A key that beats the
answer replaces the value the TUI would have extended: back to back,
`i 9 Tab Tab 5 Enter` stores 5 or 5095 in Chromium over the worker
(improvise-n4i9).
The TUI is unchanged: its `RebuildLayout` in front of `EnterEditAtCursor`
already reads the renumbered grid (gotcha 23). This also took
`CellValue::edit_text` out of the client bundle. That call formats an `f64`,
which is what compiled to `i64.trunc_sat_f64_s`, so `a093d126`'s
`nontrapping-float-to-int` flag on the wasm-client is reverted. A `?ws=` page
loads only the client bundle, so it works without the feature.
Reordering the weld to `[clear-buffer, cmd]` was tried first and rejected. Four
readers take a `Sequence`'s **first** step as its identity: `keymap::runs`,
`help.rs`, and two resolver audits. So every hint on those keys started
reporting `clear-buffer`. `text_entry_modes_clear_the_buffer_they_type_into`
keeps its guarantee by asking a plain-`Cmd` commit key's command what it emits.
`rebind` exists for
exactly one case: CommandMode's Backspace, which exits the mode on an empty
`:` line instead of popping a character that isn't there.
context/code/tooling.md
+53 -14
View File
@@ -278,7 +278,29 @@ The rule now:
exit 1.
- **A known answer ends the wait early**: a failed check (exit 1 whatever
mergeability says, since waiting would turn a known red into a timeout), or
`mergeable == CONFLICTING`.
`mergeable == CONFLICTING`. "Whatever mergeability says" includes
`MERGEABLE/BLOCKED`, which is how a failed *required* check reports here, so
it is the commonest red this script sees (improvise-9pfs).
- **A PR that is no longer open ends the wait on the first poll that sees it
closed, with exit 5** (improvise-1qaa). That poll is the first when the PR was
already merged, and can come mid-wait when someone merges it during the watch.
A merged PR reports `UNKNOWN/UNKNOWN` permanently (#189, read live), so the
UNKNOWN wait slept the whole timeout there and then blamed mergeability. A
closed PR's mergeability is whatever it was when it closed, so it can land on
either side of the wait. #74 reads `CONFLICTING/DIRTY`, which ends the loop at
once, but as a misleading exit 1. #46 has green checks and reads
`MERGEABLE/BLOCKED`, which the loop waits on, so it slept the full timeout
too (improvise-29w2). The suite carries all three shapes, and only the
waited-on ones can tell whether the state is checked at the top of the loop.
The script now fetches `state`. Exit 5 is
its own code, not 1 or 0, because under merge-as-soon-as-green "someone else
already merged it" is a normal answer, and the caller should hear it as that.
- **The head checks come first, and the ended check after them**
(improvise-24lz). A PR that merged at a head the watcher never saw is the
PR #67 hazard in a new form. It exits 2, whether the head moved during the
watch or `--expect` names another. Only then is a closed PR exit 5. On the
first poll the pin block only records the head, so an already-merged PR
still ends on its first read.
- **The pass condition is an allowlist**: `mergeable == MERGEABLE` *and*
`mergeStateStatus` in `CLEAN`, `UNSTABLE` or `HAS_HOOKS`. Anything else exits
1. That includes `null` and any state GitHub adds later, so the check fails
@@ -638,30 +660,47 @@ sixteen scripts, and the review cost is the whole diff rather than the shebang
line. The runner also does not ship zsh, so every job would need an `apt-get`.
Declined.
What was adopted instead pins the *interpreter and the environment together*, in
What was adopted instead fixes the *environment* a suite runs in, in
one place, with no per-invocation nix cost:
**`scripts/checkers.sh` is the one entry point for every check_the_checker.** CI
calls it and the local gate calls it, so there is no second invocation to drift.
It does three things a bare `./scripts/X-test.sh` does not:
It runs every suite under the bash that runs it: in CI and under direnv, that
is the dev shell's bash. It does two things a bare `./scripts/X-test.sh` does
not:
1. **Pins a bash ≥ 5.** The ambient one is tried first, so a runner that already
has bash 5.2 pays nothing; nix supplies one only when the ambient bash is
older, which on macOS outside the dev shell it is. No bash ≥ 5 and no nix is
**exit 3** — a refusal, never a quiet run under 3.2.
2. **Scrubs the environment to an allowlist** (`env -i` plus `PATH`, `HOME`,
1. **Scrubs the environment to an allowlist** (`env -i` plus `PATH`, `HOME`,
`TMPDIR`, the `NIX_*` and `GITHUB_*` names, extensible through
`CHECKERS_KEEP`). This is the part that closes the direnv hole above, and it
is a *positive* list on purpose: the failure mode of a missing name is a loud
"command not found", where the failure mode of an extra name is the silence
this section is about.
3. **Enforces the shebang rule**, so the `sh`/`bash` split cannot come back.
2. **Enforces the shebang rule**, so the `sh`/`bash` split cannot come back.
The pinned bash's directory goes on the front of `PATH`, so a script the suite
invokes through its *own* `#!/usr/bin/env bash` — or through `sh`, which
nixpkgs' bash also provides — gets the pinned one too. Pinning only the outer
shell would have left every inner one on the system bash, which is where the
fault actually lived.
**The runner no longer polices the bash version or where tools live.** It used
to pin a bash of at least 5.1 (`find_modern_bash`: try the ambient bash, fall
back to `nix shell nixpkgs#bash`, and exit 3 if neither was available). The
floor was 5.1 because `pr-reviews-test.sh` uses `wait -n -p` (improvise-fd0j).
All of that was removed on the user's ruling, when CI moved to the NixOS
runners: "we shouldn't care which version of bash and other standard utilities
are running in CI", and "most jobs should just take what comes in the ci dev
shell".
So every CI step runs inside `nix develop .#ci`, which provides the bash and the
GNU coreutils. The runner calls suites with `$BASH` and an unchanged `PATH`. A
script that needs a newer bash feature fails where it uses it, not at a version
gate.
Two things went with the floor:
- **Its self-tests.** `checkers-test.sh` sections 2, 5 and 5b faked old
bashes, or stripped `PATH` down to hand-picked utilities, to prove the floor
held. Those were the tests that broke on the NixOS runners, where `/usr/bin`
and `/bin` hold only `env` and `sh` (#296, improvise-3uow).
- **The hard-coded tool paths** in the suites (`/bin/sleep` and `/bin/date` in
`pr-reviews-test.sh`) and `scripts/kb`'s BSD `stat -f` fallback. `scripts/kb`
already needs the dev shell for `tpl`, so GNU `stat` is always present when it
runs.
**A suite is `X-test.sh` where `X` or `X.sh` is present beside it.** That rule
exists because `scripts/node-test.sh` matches `*-test.sh` and is a test
context/code/tui.md
+85
View File
@@ -424,3 +424,88 @@ is always drawn. So a browser user sees a message briefly and never loses the
hint. The TUI now does the same, on a keypress rather than a timer, because a
terminal status row has one line to share and a keypress is the event the TUI
already has.
### Except while a minibuffer stays open under it (improvise-su4u)
In a text-entry mode `minibuffer_line` draws the message to the right of the
prompt. Since improvise-v4hg, a formula that does not parse keeps the user in
`FormulaEdit` with the error. Under the plain rule, the error vanished on the
first correcting key: one Backspace and the user was fixing text they could no
longer see the complaint about. The same happened to any refusal that leaves
the user in a minibuffer.
**The rule: a key that leaves the user in the same editor, and says nothing of
its own, puts the stale message back.** `press` remembers the editor session and
the message, clears the message as before, dispatches the key, and then restores
the message only if all three of these hold:
- a text-entry mode was open before the key;
- the same editor session is open after it (below);
- the key wrote no message of its own.
So:
- editing keys keep the refusal on screen until the user acts on it;
- a key's own message always wins, so a second refused `Enter` shows the new
error, not the old one;
- leaving the minibuffer, by a successful commit or by `Esc`, drops it, exactly
as the plain rule does.
**Rejected alternatives.**
- *Expire only on the commit key.* That needs to know which keys commit, which
is the keymap's knowledge (`TextEntry::commit`), and a second copy of it in
the TUI loop. "The mode did not change" is observed rather than predicted,
like gotcha 31's `ReturnToNormalUnlessMoved`.
- *Never clear in a text-entry mode.* The message would then outlive `Esc` and
stand in the mode the user returns to, which is the cq3d bug again.
- *Compare the message after the key with the stale one.* A key that wrote the
same text would be indistinguishable from a key that wrote none. Clearing
first and restoring only an empty slot has no such ambiguity.
### What "the same editor" means (improvise-sx7h)
The first version of this rule compared modes: an equal `AppMode` before and
after the key meant the same editor. That is false for the two cell editors.
`Editing` and `RecordsEditing` commit with `CommitAndAdvance`, which drops to
Normal, writes the cell, moves the cursor and re-opens the editor through
`EnterEditAtCursor`, which sets an **equal** `AppMode`. So after an `Enter` or
`Tab` that committed and moved on, the mode looked unchanged, and the stale
message was restored beside a different cell's editor. It came back again on
every later commit until `Esc`. The reviewer's repro was `R`, `o`, `5`, `Tab`,
`7`: "Added new record row" survived the `Tab`, and so did `o`'s refusal
"That record row is already here…" after the `Tab` that filled the row in.
`FormulaEdit` could not show it, because its successful commit leaves the mode
and writes a message of its own.
An editor session is one *opening* of a minibuffer. The TUI loop cannot see
openings, so `EditorSession` compares what an opening sets:
- **the whole `AppMode`**, not only its variant: an `ItemAdd` for `Month` is
not the `ItemAdd` for `Region` it replaced;
- **the cursor cell**, which is a cell editor's target, so a commit that
advances is a new editor. The prompt modes do not move the cursor while the
user types, so for them this is inert;
- **the editor's seed** (`opened_editor.seed`). `EnterEditAtCursor` re-seeds
from the cell. So a commit that re-opens on the **same** cell, such as `Tab`
in the last column, is a new editor too, because the seed is now the
committed text.
What it cannot see: a commit that re-opens on the same cell with the same seed.
That means the user committed the text the cell already held, which writes
nothing. Restoring a message across a commit that changed nothing is harmless.
**Rejected: an editor generation counter.** It would be exact, but an editor is
opened by more than one effect (`EnterEditAtCursor`, and `ChangeMode` from
`enter-mode`). A counter bumped in each is a hand-kept list of the places a mode
is written, which is the shape gotcha 24 retired. The three fields are set by
the opening itself, so they need no list.
**Not included: the active view.** The cursor is per view, so in principle the
view belongs with the cell. But no key a minibuffer binds can switch the view
and stay in the minibuffer: a `:` line that switches view settles back to
Normal. So nothing could test the field, and an untestable field is a claim no
test can check.
**The browser is not covered by this.** There, `status_msg` is a toast that
hides after `TOAST_MS` (six seconds), and the minibuffer element draws only the
prompt and the text. A refusal therefore disappears on a timer, whatever the
user is doing. That is a different mechanism and has not been changed here.
context/test-evidence/clients.md
+299 -6
View File
@@ -213,6 +213,45 @@ when Ctrl-S became a `Request`; putting the entry back fails with *"save
stops being a gap"*. So the mechanism gotcha 47 asks for was watched failing
rather than assumed.
**Since improvise-uln5 it iterates `resolve::NEEDS_THE_HOST`** rather than a
table of its own. It derives each name's keys from the Normal and search-mode
keymaps, and arms a prefix before pressing its second key. So the list the
browser's help marks and the list this test drives are the same list.
`exit-search-mode` joined it, because search mode's `Enter`/`Esc` are silent
too. Rival: add `move-selection`, which the browser runs, to the list. This test
fails, and so does `every_help_row_the_browser_cannot_run_is_one_it_marks`.
### every_help_row_the_browser_cannot_run_is_one_it_marks
improvise-uln5. Every help row is pressed, in its own mode, on the fixture
`furnished` builds. The commands whose keys are silent must be exactly the
commands on `NEEDS_THE_HOST`, which are the rows the browser draws marked. The
check runs in both directions, because each direction catches its own mistake:
- a silent row that is not on the list is an unmarked black hole;
- a listed row that acts is a false warning.
It also asserts that no status-bar hint names a listed command, since
`hint_line` is never marked.
Rivals, each run against the fixed tree:
- Drop `search` from the list: red. This test fails, and so does
`every_advertised_command_answers_in_its_own_mode`, which no longer skips `/`.
- Add `move-selection` to the list: red. This test fails, and so does the
list's own test.
### a_help_row_the_browser_cannot_run_is_drawn_marked
improvise-uln5, the display half, and the red test. On the Navigation page the
row "Start search — …" (key `/`) must render as `dim`, with "not in the browser"
in its description. On the Commands page every `:` row must say the same. A
row the browser runs ("Jump to last row") must stay a plain `keys` row with its
description unchanged, so a renderer that marks everything cannot pass.
Red at `202e7eac`: the search row rendered as `keys`. Rival: the renderer's
`needs_the_host` check replaced by `true`. It is red here, and only here, which
is why the display has a test of its own and does not lean on the resolve-side
guard.
### a_prompt_this_client_cannot_commit_is_never_entered
The refusal list above is only half the property. `Upstream` says the key
@@ -273,11 +312,14 @@ with an empty vec, so that would be a keypress the user sees do nothing at all
### enter_commits_the_cell_advances_and_re_enters_editing
Enter in `Editing` is `[commit-cell-edit editing, clear-buffer edit]`, and every
part of that sequence shows up in what the client composes — **including the
trailing clear that empties the buffer `EnterEditAtCursor` has just seeded**.
That ordering looks wrong and is exactly what the TUI does; mirroring it line
for line is the contract.
Enter in `Editing` used to be `[commit-cell-edit editing, clear-buffer edit]`,
and the trailing clear emptied the buffer the advance had just re-seeded. That
was improvise-fy9o. Since the fix, Enter is `commit-cell-edit` alone, and the
commit clears its own buffer at its head. The test pins that order: the write,
then `ChangeMode(Normal)`, then `SetBuffer(edit, "")`, then `SetSelected`. It
ends with the re-entry, and the **last** write to `edit` is the landed cell's
`"900"`. **Rival:** `resolve::commit` without its head clear fails this test,
and `a_commit_at_the_corner_leaves_no_buffer_behind_on_both_ends` too.
---
@@ -2105,7 +2147,10 @@ two cannot drift — including prefix sequences (`gg`, `yy`, `ZZ`), which it
presses by arming the prefix first. That is what added the five host-only
black holes (`/`, `n`, `H`, `z`, Normal's `Enter`) to the list: each was already
in `the_command_names_that_still_need_the_host_are_these`, and the help page
advertised it anyway.
advertised it anyway. Since the rest of improvise-uln5 they are no longer listed
here: the browser marks those rows, and this test skips a silent row whose
command is on `resolve::NEEDS_THE_HOST`, which
`every_help_row_the_browser_cannot_run_is_one_it_marks` owns.
*Which cells.* `furnished` stood on a record cell only, so `i` on an aggregate
(s8h7's whole subject) was outside its reach. Normal mode is now pressed on a
@@ -4578,6 +4623,37 @@ a client that never writes cannot pass. The fold case,
at row 2. Rival for all three: the host's paste/clear door restored to
`ctx.cell_key()`, which was `gh/main`'s code. All three fail.
### advancing_the_editor_onto_a_cell_i_declines_leaves_both_ends_out_of_it
improvise-enz6 (and improvise-jmti), both ends. Two cases, each off the
origin, each with a control step:
- `fold_host`: `i` on Jan, `Enter` to Feb, `Enter` onto the Q2 fold at row 2.
- `yank_host`: `i` on North/Revenue, `Tab` to Cost, `Tab` onto Profit at
column 3.
After every key the client's mode, cursor and buffers must equal the host's.
At the end, neither end may be editing, and the browser must have said
something.
It was red with the host fixed and the browser not. At step 2 the client was
`Editing` and the host `Normal`. So this is the test that keeps the two ends'
re-entry in step. Before the fix they agreed, both opening the editor, and
nothing compared them on a cell `i` declines.
The status text is not compared. On a fold the host names the collapsed group,
and the browser, which sees only `CellId::Aggregate`, gives the generic
aggregate text (`context/code/clients.md`).
Rivals:
- The browser's `reopened_at` opening everywhere: fails at the fold's step 2.
- `reopened_at` opening on an aggregate only: fails on the fold.
- The host opening everywhere: fails, the client now declining where the host
opens.
One limit is not covered, because the window is 6×6 and every landing is
inside it. A landing past the announced window has no projected id, so the
browser still opens the editor there (`clients.md`).
### the worker is not called up until its first frame has arrived
improvise-i0v9. The worker transport wrote `in-page worker · <doc>` when it
@@ -5099,6 +5175,75 @@ is touched. Enter must then land on (2, 0), and the folded `opened_editor` must
be exactly `OpenedEditor::on("800")`. **Injection:** `resolve::advanced`
carrying the previous editor's `touched` flag. This test fails alone.
### `enter_enter_leaves_every_cell_unchanged_on_both_ends`
**improvise-fy9o**, browser and host in lockstep. On the 2×2 `precise_host`,
starting at (South, Cost), row 1 and column 0, the test presses `i`, Enter,
Enter. The client resolves each key, folds it and applies it to one host, and
`reduce_key` drives a second host. After each key the client's buffers must
equal the key host's, and at the end every cell must be unchanged on both
hosts.
Failed before the fix: (North, Revenue) was cleared, because the welded clear
had blanked the reopened editor. **Rival:** the editing commit keys bound
through the welding `.commit` again fails this test, the typing test below, and
both TUI tests.
### `typing_into_the_reopened_editor_extends_the_cell_on_both_ends`
The other half of the ruling: the reopened editor shows the landed cell, so
typing extends it. `i`, Enter, `5`, Enter must store `25` at (North, Revenue)
on both ends. It fails under the weld rival, because the digit went into a
blank buffer and stored `5`.
### `a_commit_at_the_corner_leaves_no_buffer_behind_on_both_ends`
Moving the clear from the keymap into the command must not leave text behind
where nothing reopens. At the bottom-right corner, `i`, `9`, Enter must leave
`edit` as `""` on both ends, with the mode no longer editing. **Rival:** the
browser without `cleared` leaves `"49"` and fails here. The equivalent TUI rival
fails `text_entry_modes_clear_the_buffer_they_type_into`.
### `tab_at_the_rightmost_column_reopens_on_the_value_just_written`
**improvise-3tt4**, found by the #227 review at `a1b23c05`. `resolve::advanced`
re-seeded the reopened editor from the client's cache, on the argument that the
advance always lands on a *different* cell. That is false for Tab at the
rightmost column: `Advance::Right` clamps back onto the cell the frame has just
written, and that cell's cache entry is the pre-write value.
The test uses the lockstep 2×2 fixture and starts at (0, 1), which is North /
Revenue and holds 2. It presses `i 9 Tab 5 Enter` on a browser client and on a
key-driven host, and compares the buffers after every key. It failed at
`a1b23c05` on `Tab`: the browser buffer was `"2"` and the host's was `"29"`.
Pressing `5 Enter` would then have written 25 over the 29 the user had just
committed. Both ends must end at 295.
**Fix, superseded by improvise-3uzg:** the first fix, `landed_seed`, seeded
from the committed text on the cursor's own cell. It assumed that cell was the
only stale one, which prune disproved. After a write the editor now awaits the
host's answer, so this test drives each key through `sent_and_answered`: the
key is folded as an outgoing frame, the host applies it and projects, then
`Accepted` arrives. Both rivals below still fail it.
### `tab_at_the_rightmost_column_reopens_on_the_stored_spelling`
This pins what "the committed value" means: the stored value's edit text, not
the raw typed string. `i Backspace 0 0 7 Tab` at (0, 1) must leave `"7"` in both
buffers. **Rival** (under the 3tt4 fix): seeding from the raw text failed with
`"007"` against `"7"`. Under the 3uzg fix the seed comes from the host's
projection, which has only the stored value to give. Opening the editor at once
from the cache fails this test.
### `a_category_committed_at_plus_cat_leaves_no_buffer_behind_on_both_ends`
**improvise-5wai**, found by the #227 review. `+cat` reopens nothing, so the
commit's own head clear (`resolve::cleared` in `create_column`) is the only
thing that empties the buffer. The test types `Season` at the `+cat` column of
the records pair, presses Enter on both ends, and requires the category to have
landed and both buffers to be `""`. **Rival:** `let _ = cleared(&edit_mode)` in
`create_column` fails only this test, with the browser keeping `"Season"`.
### an_edit_opened_before_a_document_swap_is_refused_in_the_new_document
improvise-vndi, through a real `Host` over `MemoryFs`. A opens an inline editor
@@ -5117,3 +5262,151 @@ Rival injected: `claim_as_read` ignoring the captured generation and claiming
the current one. That is the pre-fix behaviour, restated after the fix. Only
this test failed in the whole workspace. The layout tier staying current is
still pinned by `an_edit_writes_the_cell_the_user_clicked_after_the_rows_renumber`.
### `a_commit_that_prunes_its_row_reopens_on_the_row_that_moved_up`
**improvise-3uzg**, found by the #227 review at `a093d126`. Fixture:
North/South/West = 1/2/3 on one measure, `prune_empty` on, window 3×3. Keys
`i Backspace Enter 5 Enter`, each through `sent_and_answered` on the browser and
through `reduce_key` on the TUI host. The buffers must agree after every key,
and the two documents must be identical at the end. The Backspace-Enter empties
North, which prunes its row, and the advance lands on row 1, which is now West.
Red at `a093d126`: after the first Enter the browser buffer was `"2"` (South, the
stale cache) and the host's was `"3"`. At gh/main both reopened blank.
**Rivals** (each fails this test and the two `tab_at_the_rightmost_column_*`
tests):
- `settle_awaited_seed` not waiting for unanswered frames, so it seeds while
folding its own outgoing frame.
- `accepted()` not settling, so the seed never arrives.
- The resolver seeding from the cache immediately after a write.
### `client::tests::an_awaited_seed_waits_for_the_answer_and_never_overwrites_typing`
The fold half of improvise-3uzg, without a host. The steps:
1. A write frame with an awaiting editor is sent over a cache that says `2`.
2. The host's projection saying `3` arrives. The buffer must still be empty.
3. `Accepted` arrives. The buffer and the opened editor must be seeded `3`.
4. In a second client, a keystroke `5` arrives before the answer. After
`Accepted` the buffer must still be `5`.
**Rivals:**
- Seeding while a frame is unanswered fails this test and the three lockstep
tests.
- `accepted()` not settling fails the same four.
- Ignoring `touched` fails this test alone. It is the one guard on type-ahead.
### `a_tab_sent_before_the_last_write_is_answered_reopens_on_that_write`
**improvise-mdfc**, found by the #227 review at `baabe1a9`. Fixture:
`precise_host`, cursor (0,1) on North/Revenue = 2 (the rightmost column), window
2×2. `i 9` go through `sent_and_answered`. Then **three** Tabs are sent through
`sent`, and none is answered until all three are out. The first writes 29 and
the other two write nothing. After the answers are delivered in order, the
browser's buffers must equal the TUI host's. `5 Enter` then has to store 295 on
both ends.
Every earlier lockstep test answered each key before the next was sent, so the
client's cache was always the host's by the time it resolved a key. That is why
none of them saw this. The hazard needs a key resolved while a write is still
unanswered.
Red at `baabe1a9`: after the answers the browser buffer was `"2"` (the cache from
before the write) and the host's was `"29"`. In Chromium over the worker, with
`tall.improv` R01/Cost = 50 and `i 9 Tab Tab 5 Enter`, the browser stored 505
where the TUI stores 5095.
**Rivals:**
- The fix reverted (await only when this commit writes) fails this test alone.
The workspace was otherwise green at `baabe1a9`.
- Awaiting only when the **newest** in-flight frame is awaited
(`in_flight.back()`) fails this test, and the other 53 lockstep tests pass. That is why it sends three Tabs,
not two: with two, the newest in-flight frame *is* the write, and `back()` and
`any()` cannot be told apart.
### `client::tests::an_awaited_seed_arriving_after_escape_leaves_the_buffer_empty`
**improvise-4cb5**. A write frame with an awaiting editor is sent, and then Esc
(`SetBuffer edit ""`, `ChangeMode Normal`) is sent before the answer arrives.
The host's projection says `3`, and then `Accepted` arrives. The mode must be
`Normal` and the edit buffer must be empty.
Red before the fix: the buffer was `"3"`. `settle_awaited_seed` asked only
whether the opened editor was awaiting and untouched, and Esc leaves
`opened_editor` in place. The seed `3` is distinct from the empty buffer Esc
leaves, so the assertion can only pass if nothing was written.
**Rival:** the mode check removed (the code at `baabe1a9`) fails this test, and
the rest of `improvise-protocol`'s lib passes.
### `resolve::opened_editor_wire_tests::a_frame_whose_editor_lost_its_awaiting_flag_does_not_parse`
**improvise-mgis**. A `ClientFrame::Commands` carrying
`SetOpenedEditor(Some(OpenedEditor::awaiting()))` is serialized, and the
`awaiting` key is removed from the JSON. The test asserts the key was there, and
that `seed` is still there, so the mutated value is a well-formed editor minus
one field. It must then fail to parse **as the missing field**, not as broken
JSON.
**Rival:** `#[serde(default)]` restored. The frame then parses, with
`awaiting: false`, and the test fails at `expect_err`. Before this test existed,
deleting the attribute left the whole workspace green, so nothing pinned it.
### `an_editor_opened_with_i_before_the_last_write_is_answered_opens_on_that_write`
**improvise-p6v6**, found by the #227 review at `de4a7d78`. It predates #227:
the reviewer reproduced it against gh/main `202e7eac`. Fixture: `precise_host`,
cursor (0,1) on North/Revenue = 2 (the rightmost column), window 2×2.
1. `i 9` are answered one at a time.
2. `Tab` (writes 29), `Escape` and `i` go out through `sent`, and none is
answered until all three are out.
3. The browser's buffers must then equal the TUI host's.
4. `5 Enter` must store 295 on both ends.
Red at `de4a7d78`: the browser buffer was `"2"` where the TUI's was `"29"`. `i`
reaches `edit_or_drill`, which called `opened_on(cell.edit_text)` with no
in-flight check. mdfc had put that check inside `advanced` alone.
**Rivals:**
- **The fix reverted** (the code at `de4a7d78`): fails this test only.
- **`advanced` defers wholly to `opened_on`** (its own `writes` half dropped,
on the reasoning that `opened_on` now checks): fails
`a_commit_that_prunes_its_row_reopens_on_the_row_that_moved_up`, both
`tab_at_the_rightmost_column_*` tests, and
`resolve::tests::enter_commits_the_cell_advances_and_re_enters_editing`. The
commit being resolved is not in `in_flight` yet, so `opened_on` cannot see its
write.
### tab_in_an_editor_commits_and_lands_one_column_right
improvise-rq3f, found in the adversarial review of PR #246. Changing `advanced`'s
`Advance::Right` arm to `Some((row, col))` passed the whole workspace (1945
tests): nothing asserted where the browser's Tab-commit
(`commit-and-advance-right`) lands. The test opens the editor with `i` on a
centred cursor, (1, 1) in a three-by-three grid, so no edge clamps the move.
It then presses Tab and asserts three things: the composed commands contain
`SetSelected { row: 1, col: 2 }`, the folded cursor is at (1, 2), and the
editor is open again there.
- **Rival `right_stays`** (the ticket's fault): fails only this test, across
the workspace.
- A unit test and not a lockstep case, because the fault is in the resolver's
own arithmetic. Lockstep compares the client with the host; this test
compares the client with the answer `CommitAndAdvance` gives, whose
`AdvanceDir::Right` arm is `(col + 1).min(col_max)`.
### the_tile_cursor_stops_on_the_last_tile
improvise-rq3f. Changing `tile_cursor`'s upper clamp from `(count - 1)` to
`count` passed the whole workspace: pressing right on the last tile selected an
index past the end, and nothing noticed. The fixture is `furnished(TileSelect)`,
with three tiles and the cursor on the middle one. The control moves right
once, unclamped, to the last tile. Then both `Right` and `l` must clamp there,
and each must still emit `SetTileCatIdx(last)`, as `MoveTileCursor` emits even
when the index does not move (gotcha 47).
- **Rival `clamp_count`** (the ticket's fault): fails only this test, with
`SetTileCatIdx(3)` over three tiles.
- The control matters: without the unclamped move first, a resolver that never
moves at all would also pass the two clamped presses.
context/test-evidence/command.md
+290 -10
View File
@@ -227,6 +227,14 @@ checked by adding a `probe_field` to `Model` and to `Model::new`. The build then
failed with `E0027: pattern does not mention field probe_field` at the `Debug`
impl, and nowhere else.
**`Model.measure_agg` needed the same treatment (improvise-m6op).** `Model`'s
`Debug` sorts `measure_agg` through a `BTreeMap`, but the fixture's
`measure_agg` was empty, so nothing saw the sort. With the sort replaced by the
raw `HashMap` (`.field("measure_agg", &measure_agg)`), this test stayed green in
3 of 3 runs. The fixture now carries four entries: `Revenue` Avg, `Cost` Min,
`Units` Max and `Orders` Count. Under the same injection it is red in 3 of 3
runs, and it is green with the sort restored.
### a_stale_wire_model_command_cannot_be_applied
A wire `Command::Model` cannot reach the document without its basis being checked
@@ -2883,6 +2891,11 @@ The buffer is derived from each mode rather than restated in the test —
`every_text_entry_mode_types_into_the_buffer_its_prompt_names` owns pinning what it
should be (improvise-k8h).
**Amended by improvise-fy9o.** A commit key bound to a plain command now passes only if that command, built in
the mode with text in the buffer, emits `SetBuffer { name: <buffer>, value: "" }`
itself. **Rival:** `CommitAndAdvance` without its head clear. Only this test
fails, which makes it the TUI's guard against leftover text.
### minibuffer_enter_dispatches_to_a_registered_commit
improvise-6t3: the three commit commands collapsed into a single
@@ -4534,6 +4547,8 @@ Two faults injected:
assertion (`left: Some("0.50") right: Some("0.5000")`).
- Dropping only the `col_formats` write in `viewport::grid_body`, leaving the
cells correct, fails the totals assertion (`left: "1.50" right: "1.5000"`).
Those numbers predate improvise-wm20. The total is now the formula over the
pooled rows, `0.5000`, and the same rival reads `0.50`.
The second is the one that earns the test its length: without it, the totals
strip could round every formula column to the view's format and no assertion in
@@ -5344,11 +5359,67 @@ deliberate over-refusal, taken to keep one predicate.
### enter_advancing_onto_a_fold_commits_nothing_there
The write half of improvise-enz6, closed by the commit door. `i` on Feb, then
`Enter`, which commits and advances onto the Q2 fold and re-opens the editor
there. That entry is still enz6's open half. Then `7` and `Enter`. At the
previous commit door (`cell_key`) the second commit stored `Month=Q2`. Rival:
`commit_at_cursor` on `cell_key` again. This test and
`tab_advancing_onto_a_formula_cell_commits_nothing_under_it` fail.
`Enter`, which commits and advances onto the Q2 fold. When this test was
written, the advance also re-opened the editor there. That was enz6's open
half, and `enter_advancing_onto_a_fold_does_not_reopen_the_editor_there` now
closes it. Then `7` and `Enter`. At the previous commit door (`cell_key`) the
second commit stored `Month=Q2`. Rival: `commit_at_cursor` on `cell_key` again.
This test and `tab_advancing_onto_a_formula_cell_commits_nothing_under_it` fail.
**That rival no longer fails either test**, measured after enz6's entry half
closed. The advance now leaves the user in normal mode on the fold, so `7` and
`Enter` never reach the commit door. Both tests still pin the end-to-end
property that the key path writes nothing there. The commit door itself is now
pinned by `an_editor_already_open_on_a_fold_commits_nothing_there` and its
formula twin, which that rival fails. The door is still reachable: a
`:commit-cell-edit` line runs it at the cursor, and a browser editor can open
past its window (`context/code/clients.md`).
### an_editor_already_open_on_a_fold_commits_nothing_there
The commit door alone, once improvise-enz6 closed the route the tests above
reached it by. The test seats the editor on the fold directly: mode `Editing`,
buffer `7`, cursor on the fold at (2, 1). Then `Enter`. Nothing may reach `## Data`
with `Q2`, and the status names the collapsed group. Rival: `commit_at_cursor`
back on `cell_key`. This test and the formula twin fail, and the four
advance-path tests pass.
### an_editor_already_open_on_a_formula_cell_commits_nothing_under_it
The same door on `formula_app`'s Profit (row 1, column 2): `7` then `Enter`
stores nothing under the formula. The rival is the one above.
### enter_advancing_onto_a_fold_does_not_reopen_the_editor_there
The entry half of improvise-enz6. `CommitAndAdvance` ends in `EnterEditAtCursor`,
which opened the editor on whatever cell the advance landed on. `i` declines a
fold, and the re-entry did not. The fixture is `app_on_a_fold`, whose fold sits
at row 2, off row 0. `i` on Jan (row 0, column 1), then `Enter` to Feb. That
first step is the control: Feb is an item, and the editor must re-open there,
so a re-entry that never opens cannot pass. The second `Enter` lands on the
fold, and the test asserts the mode is not editing and that the status names
the collapsed group.
It was red at `202e7eac` (gh/main) on the mode assertion: `Editing`. A first
version also asserted that `opened_editor` was `None`. It failed after the fix,
because Feb's `OpenedEditor` outlives the commit. Nothing clears it in normal
mode, and every opener overwrites it, so that is not this bug. The assertion
was dropped rather than widening the fix.
Rivals:
- `EnterEditAtCursor` opening everywhere (the `uneditable` check replaced by
`None`): this test, its formula twin and the lockstep test fail.
- `uneditable` answering `None` for a formula's result: the formula twin,
`tab_advancing_onto_a_formula_cell_commits_nothing_under_it` and the lockstep
test fail. That confirms the entry and the commit door ask one function.
### tab_advancing_onto_a_formula_cell_does_not_reopen_the_editor_there
enz6's entry half on the second arm `i` declines, a formula's result. This is the
case improvise-jmti filed separately. `formula_app`, `i` on West/Revenue (row 1,
column 0). `Tab` to Cost is the control, and the editor re-opens there. The
second `Tab` lands on Profit, and the test asserts no editor and the formula's
refusal text. It was red at `202e7eac` on the mode assertion. Rivals: as above.
### x_in_records_mode_clears_the_real_record_under_the_cursor
@@ -5380,7 +5451,10 @@ A door the tickets did not list, found by working the enumeration. `i` declines
to edit a formula's cell, but `Tab` from West/Cost commits and advances onto
Profit, `EnterEditAtCursor` re-opens the editor there, and `Enter` stored
`7` under the formula. Found by this test failing at `90b93d4`, before
`committable_cell_key` existed. The rival is the one above.
`committable_cell_key` existed. The rival is the one above. Since
improvise-enz6 the advance no longer opens that editor, so this test is
end-to-end only and the rival passes it. The door is
`an_editor_already_open_on_a_formula_cell_commits_nothing_under_it`.
### ui::host::tests::every_failing_model_command_refuses_its_whole_frame
@@ -5919,8 +5993,10 @@ is stored, so the stored arm accepted it without asking the projection. The test
runs with `Money` collapsed, where the host draws that cell as the fold, and
expanded, where the host draws no such cell. It asserts `MutationRefused` and an
unchanged document in both, so a rule that asks the host's layout cannot pass
the expanded half. Red at `5eaafbae`. The fix is `App::group_refusal`, which asks
the document whether any coordinate names a group rather than an item. Rival:
the expanded half. Red at `5eaafbae`. The fix was `App::group_refusal`, which
asked the document whether any coordinate names a group rather than an item.
Since improvise-nacd it is `App::non_item_refusal`, which asks whether every
coordinate names an item. Rival:
the group rule removed. Injected into a full workspace run, it fails this test
and nothing else.
@@ -5941,6 +6017,72 @@ improvise-b114 rule is: no coordinate names a group, asked of the document.
Rival: the group rule removed. Injected into a full workspace run, it fails this
test and the b114 test, and nothing else.
### a_wire_write_to_an_item_named_like_its_categorys_group_is_accepted
improvise-pr8t, found in the adversarial review of PR #235 at `5c19fdd4`.
`Category::add_item` checks only the items, so a category may hold an item
`Money` and a group `Money` at once. The b114 rule exempted that case with
`!category.items.contains_key(name)`, and nothing pinned the exemption: with it
removed, the whole workspace stayed green while a caught-up write to
`{Month=Feb, _Measure=Money}`, a real item's cell, was refused.
The test builds exactly that case on the stored record `{Month=Feb}`. It asserts
`Accepted` and the stored value, so a rule that refuses and a rule that accepts
but drops the write both fail.
The test passed on the unfixed code, as a pin should. Two rivals were run against it:
- On the b114 code, the exemption removed (`let group = true && …`): red,
`a real item was refused: … Rejected(MutationRefused)`.
- On the nacd code, a rule that also refuses any name that is a group, even
when it is an item: red, and in a full `improvise-command` run this test is
the only failure.
### a_wire_write_naming_no_item_of_its_category_is_rejected
improvise-nacd, found in the same review. The b114 rule refused a coordinate
that *is a group*. The invariant it protects is wider: every coordinate names an
item (improvise-gw18). Two writes fell in the gap. The fixture puts Revenue and
Cost in `Money`, puts `Money` under the parent group `Totals`, and stores
`{Month=Feb}`.
- `_Measure=Bogus` is no item and no group.
- `_Measure=Totals` is a parent group that holds no item directly, so
`items_in_group("Totals")` is empty and the group test did not see it.
Both were answered `Accepted` and stored the coordinate. The test asserts
`Rejected(MutationRefused)` and an unchanged document for each. Both were
confirmed red separately at `202e7eac`: the loop stops at its first failure, so
the second name was checked by swapping the order.
The fix is `App::non_item_refusal`: every coordinate must be one of
`Model::effective_item_names` of its category. The argument for a positive rule
is in `context/code/command.md`, in the section on wire writes.
Rival: restrict the rule to `_Measure`, the ticket's first suggested fix shape.
It fails `a_wire_write_to_a_fold_record_with_an_undrawn_measure_is_rejected`,
because `Month=Q2` is a group of a regular category.
The fix also moved two fixtures in `ui/reduce.rs`. `model_app` and
`a_wire_model_command_marks_the_document_dirty` stored or wrote
`_Measure=Revenue` without adding `Revenue` as an item. So the cell they wrote
was exactly the non-item coordinate this rule refuses. Both now add the item.
That changes nothing they assert. It is the fixture being made a document that
could exist.
### a_write_under_a_formula_is_refused_for_the_formula_not_as_a_non_item
improvise-nacd. A formula target is a `_Measure` name but not an item of the
category, so the non-item rule could refuse a write under a formula before the
`Derived` rule saw it. The write would still be refused, but for a reason that
is false. The rule uses `Model::effective_item_names`, which includes formula
targets, so the formula rule gives the reason.
Rival: the rule written with the category's raw `items`. Before this test, that
rival passed the whole workspace, because every other formula test asserts only
that the write is refused. It fails this test with
`refused for another reason: Profit is no item of _Measure, …`.
### a_stored_record_the_hosts_view_aggregates_is_writable
improvise-sjdb, from the #217 review. South/Revenue is stored, then `Product` is
@@ -6098,8 +6240,9 @@ The buffer after `Enter` is **not** asserted. The keymap welds `clear-buffer`
after every commit key, so it empties the buffer `EnterEditAtCursor` has just
seeded, and the reopened editor opens blank while its captured seed is `"2"`.
That is existing behaviour (`enter_commits_the_cell_advances_and_re_enters_editing`
in `clients.md`). An `Enter` there with nothing typed therefore clears the landed cell,
measured here; that is filed as improvise-fy9o rather than fixed here.
in `clients.md`). An `Enter` there with nothing typed therefore cleared the landed
cell. It was filed as improvise-fy9o and is now fixed (see
`enter_enter_leaves_every_cell_unchanged`).
### `an_editor_typed_back_to_its_seed_still_writes`
@@ -6156,6 +6299,66 @@ flag. This test fails alone. Every other test stays green, which was the gap.
Once improvise-fy9o re-seeds the reopened editor, a stale `touched` would make
every untouched Tab or Enter write its seed.
### `enter_enter_leaves_every_cell_unchanged`
**improvise-fy9o.** The keymap welded `clear-buffer` after every commit key, so
it blanked the editor `CommitAndAdvance` had just reopened. The next Enter then
committed `""` and cleared the landed cell. The test presses `i`, Enter, Enter
on the 2×2 fixture:
- in pivot, from (1, 0);
- in records, from (1, 2), the West row's Cost column.
Every cell must be unchanged. Both cases failed before the fix, clearing
(North, Revenue). The records case was confirmed on its own, because the loop
stops at the first failure. **Rival:** the editing keys bound through the
welding `.commit` again fails both.
### `typing_after_the_reopen_still_writes`
After `i` and Enter from (1, 0), the reopened editor must hold `"2"`, the value
of (North, Revenue). Typing `5` and pressing Enter must then store `25`. Before
the fix the buffer was `""`, so the typed digit replaced the value instead of
extending it.
## `ui/project.rs` — the totals strip (improvise-wm20)
### a_ratio_columns_total_is_the_ratio_of_the_totalled_inputs
improvise-wm20. `Margin = (Revenue - Cost) / Revenue` over North (10, 5) and
South (30, 3). The rows read 0.50 and 0.90, so the three candidate rules give
three different answers: summed **1.40**, averaged 0.70, and pooled
(40 − 8) / 40 = **0.80**. A fixture whose rows share a ratio cannot tell them
apart; the old `a_formulas_format_reaches_its_own_column_and_no_other` is one
such fixture. **Red at `60179150`** with 1.40. Rival: the red commit itself,
where the walk sums.
### an_average_measures_total_is_the_mean_over_its_records
improvise-wm20. `Price` under Avg, with `Product` on None, so North's row is the
mean of two records (10, 20) and South's of one (30). Summing the rows gives
**45**, the mean of the rows' means 22.5, and the mean of the records **20**.
The ruling is the last. **Red at `60179150`** with 45.
### a_hidden_rows_values_stay_out_of_the_total
improvise-wm20. `East` (100, 100) is hidden, so both the Revenue total (40) and
the pooled Margin (0.80) must leave it out. Pooled in, the Margin would read
32/140. This test earns its place through two rivals, each injected into a full
workspace run where each fails this test and nothing else:
- The span ignores `covers`, so hidden rows are pooled in.
- A formula's references are read **unspanned**. The ratio test cannot see this
one: an unspanned reference reads the column key over every record, which is
the right answer whenever nothing is hidden. Only a hidden row separates
"every drawn row" from "every record".
### a_max_measures_total_does_not_move_when_a_row_group_collapses
improvise-wm20, from the #203 re-review's comment on the bead. `Revenue` under
Max, with Region on None and Month's Jan and Feb in Q1. Expanded, the rows are
5 and 7, which a summing strip reads as **12**. Collapsed, the fold reads 7. The
total must be 7 both ways. **Red at `60179150`** with 12.
#### improvise-i3o5: four declines that `set_status` hid from every sweep
`every_refusal_site_refuses_its_failure_and_only_its_failure` gained four cases.
@@ -6185,6 +6388,29 @@ runs `cargo test --workspace --no-fail-fast`. Four of four fail this test, and
nothing else fails. None of the four needs an abort: each is the only effect its
command emits, and `p`/`i` are single bindings, not sequences.
### a_formula_rows_error_does_not_poison_the_pooled_total
improvise-b5dp, the user's ruling on the conflict between wm20's rulings (1) and
(2): a formula column's total is the **pooled ratio**. A row's formula error
does not poison it, and a stored error still does
(`an_error_in_any_row_makes_the_total_an_error`). Rows North 25/30, South 27/30
and East -4/0 draw 0.83, 0.90 and `div/0`. The total must read **0.80**
(48/60), not an error. The test asserts that East's row really draws the error,
so a fixture where no row errors cannot pass it.
It was **green at `9948aaf5`**, because #239 already implemented the choice the
user then ratified. It is a pin, not a reproduction. Rival: in `grid_body`'s
walk, a drawn cell's `Error` overwrote its column's total. That is ruling (2)
applied to every row. This test and its Share sibling fail on the total
assertion. The other seven `column_totals` tests pass.
### a_share_columns_total_pools_past_a_rows_div_zero
improvise-b5dp, the second case the bead named. `Share = Revenue /
SUM(Revenue)`: East's own `SUM` is 0, so its row draws `div/0`. The pooled
total is 60/60 = **1.00**. It was green at `9948aaf5` and fails under the same
rival as the test above.
### a_frame_that_reframes_and_then_places_the_cursor_leaves_the_asker_placed
improvise-wdmp. A browser frame that reframes the grid and then places the cursor,
@@ -6208,3 +6434,57 @@ case fails. Second, the ticket's suggested fix, which drops the asker's
reset and the client never heard. That rival also fails
`a_geometry_command_relays_the_cursor_reset_to_the_asker_too` and the
ws-server's `the_other_document_would_also_have_a_cursor_to_invalidate`.
## `ui/host.rs` — the abort arm of `Host::mutating_frame` (improvise-we1a)
### every_failing_model_effect_that_aborts_also_refuses
What the gap was: `Host::mutating_frame` rejects a frame when
`last_batch_refused() || last_batch_aborted()`. Every model effect that aborted
also refused, so deleting the `|| last_batch_aborted()` arm left all tests
passing. The wire table (`every_failing_model_command_refuses_its_whole_frame`)
cannot hold the invariant, because that arm rejects a frame that aborted
without refusing. The test applies each failing `ModelCommand`'s own effects,
from `model_effects`, straight to an `App`, and asserts that the batch refused.
Each case also carries whether it aborts. That catches a table in which nothing
aborts, and a site that quietly stops aborting.
Faults injected:
- `AddCategory`'s failure arm set `status_msg` and aborted, but did not refuse.
- This test failed, and so did the key-path table.
- The wire table passed, which is the gap.
- With the arm also deleted, `a_refused_mutation_rolls_back_its_predecessor_and_the_abort_flag`
failed as well. So the arm is what makes the wire table look correct under
this fault.
- `refuse_and_stop` wrote the status and aborted without refusing. This test
failed, as did the key-path table,
`a_failed_load_or_import_does_not_run_the_quit_after_it` and
`a_formula_that_does_not_parse_keeps_the_user_in_the_editor_with_their_text`.
The wire table passed.
What this table cannot see (improvise-lasd, found in review of #229): a site it
does not list. The fault injected was `on_view`'s missing-view branch changed to
write the status and abort, without refusing. The workspace stayed green, and
only `Host::mutating_frame`'s `|| aborted` arm caught it. That is why
`App::apply_effects` now carries a `debug_assert!`: a `Model` effect that aborts
must have refused. Under the same fault, the assertion fails two existing tests:
- `a_frame_writing_a_view_its_own_first_command_deleted_is_refused`, with
`SetNumberFormat(Named("Scratch"), ",.2f")`;
- `a_command_naming_a_view_the_document_has_not_got_writes_nothing`, with
`SetAxis { view: Named("NoSuchView"), .. }`.
With the assertion removed, the same fault fails nothing.
### every_refusal_site_refuses_its_failure_and_only_its_failure — three cases (improvise-rtud)
This entry covers the rows `load of an absent file`, `import-json of an absent
JSON` and `a formula that does not parse`. Each has a succeeding twin
(`load doc.improv` from `MemoryFs`, `import-json good.json`, `Total = 1`).
Each row was checked by a rival at its own site: the `refuse_and_stop` call in
`AddFormula`'s parse arm, in `LoadModel`, or in the JSON read arm of
`ImportJsonHeadless`, replaced by a status write plus the abort. Under each
rival, the table fails at that row. The matching row of
`every_failing_model_effect_that_aborts_also_refuses` fails too. A rival in
`refuse_and_stop` itself is no use here, because the table stops at its first
failing row, which is the older `import-json of an absent CSV`. The rtud prose correction is landing separately, so these
rows are coverage only.
context/test-evidence/core.md
+67
View File
@@ -1511,3 +1511,70 @@ are each compared with the same document with Month on `Axis::None`. At
`Flag` at `Some(0.0)`, false rather than absent, so a fix that dropped the cell
cannot pass. Rival: `read_coordinate` ignoring the reach (`if false && …`).
This test fails.
### a_fold_row_evaluates_each_formula_once_across_the_row
improvise-h6z1. `F0 = Revenue`, `F{i} = F{i-1} + F{i-1}` up to F8, as columns,
with Q1 collapsed so the Q1 row is a fold. The test evaluates every cell of that
row twice and compares the two runs: once with a fresh memo per cell (`evaluate`),
where each column re-derives the chain below it (1 + 2 + … + 9 = **45**), and
once with one memo shared across the row (`evaluate_in`), which gives **9**. It
also asserts the two runs produce identical values, so a memo that returned a
wrong entry cannot pass. The 45 is asserted as a setup, so a fixture whose chain
had collapsed into one column could not pass the 9 by accident. Rival:
`evaluate_covering_in` ignoring the memo it is given and making a fresh one.
Injected into a full workspace run, it fails this test (45 against 9) and
nothing else.
### a_spanned_scan_finds_what_a_lookup_per_member_finds
improvise-yb4b. `fold_stored` used to enumerate one `CellKey` per member of a
fold and look each one up. Since #239 a column total spans every row
category, which is 3,000 lookups for one 18k column, paid twice per keypress.
`DataStore::spanned_values` now answers with one index-narrowed scan. This test
compares the scan with the per-member definition it replaces, in both modes, on
a store built to separate them. It has a record with an **extra coordinate**:
`get` misses it and `matching_values` finds it. It has an **uncovered item**, a
record **lacking** the spanned category, **another measure**, and another
value of an unspanned coordinate. It also has a covered item that was never
interned. The values are distinct powers of two, so a sum identifies the
admitted set. The order differs on purpose: the store's order, not member
order, as gotcha 18 names it.
Three faults were injected, each run against the full `improvise-core` and
`improvise-command` suites:
- Exact mode loses its length check (`==` → `>=`). **Only this test fails.**
Nothing else in either suite reaches an `Exact` fold over a record with an
extra coordinate.
- The span ignores coverage (`|| true`). This test fails, and so do five others
(collapsed-group, hidden-row and CSV-agreement tests).
- A category the store never interned is skipped instead of matching nothing.
**Only this test fails.**
### a_span_finds_what_its_members_find_in_store_order
improvise-jq0m. `spanned_values` now chooses between two narrowings, and a
fixed example can only ever take one of them. This property builds random
stores over `R`, `P` and `M`. Some records lack `R`, some carry an extra `X`,
and `r6` is a covered item the store never interned. It spans `R` and sometimes
`P`, in both modes, and compares the result with the per-member definition,
**in store order**: the values the members' own lookups find, read back in
`iter_cells` order. Store and span sizes vary across cases, so both narrowings
are taken.
Three faults were injected, each run against the full `improvise-core` and
`improvise-command` suites:
- The per-item narrowing drops the base, so it keeps each item's pair alone.
This test and `a_spanned_scan_finds_what_a_lookup_per_member_finds` fail.
**Nothing in either crate's grid or command tests does.** Those fixtures never
hold a second value of the base's coordinate within a small span.
- The base narrowing drops the base. This test fails, along with eight
fold and totals tests.
- The per-item positions are not sorted after they are merged. **Only this
test fails.** Values stay correct and only their order moves, which changes
a Sum or Avg in the last bits.
The regression itself is a cost, which no test here can see. The evidence for
it is the `grid_projection` bench's grouped cases, added for this ticket.
context/test-evidence/formula-and-io.md
+35
View File
@@ -1197,6 +1197,41 @@ a further 0.39s → 0.21s but it is not the fix, and reporting it as one would
be the mistake this entry exists to record. Net: 1.42s → 0.21s, and
`improvise-io` is no longer the run's longest binary.
### parser::generator::the_grammar_is_parsed_once_however_many_formulas_the_strategies_yield
improvise-i5z6 is improvise-8shu's twin, in `improvise-formula`. Both
generator strategies, `formula_string()` and `expr_string()`, called
`load_grammar()` inside their `prop_map`. So `pest_meta` re-parsed
`formula.pest` for every generated case: three 256-case properties plus
shrinks. The `improvise-io` fix never reached this crate, which has its own
copy of the generator.
All figures were measured on an Apple M4 Max (16 cores) under heavy parallel
load (load average 15–33), because an unloaded window was not available. The
ratios are more trustworthy than the seconds.
- **Before:** the lib binary was the **longest binary in all three workspace
runs**, at 0.91s, 1.02s and 1.95s. The runner-up was 0.50–0.92s, and a fourth
run gave 3.13s. The median binary reports `0.00s` at two decimals, so the
ratio to the median is not defined. By any finite reading it is two orders
of magnitude above the median. Alone, the binary took 0.84–0.98s over five
runs. The three generator properties alone took 0.94–1.03s, and the other 82
tests together took 0.01s.
- **After:** the binary alone takes 0.06–0.07s over five runs, and
0.06–0.09s inside the workspace run. `headless_cli`, which spawns processes,
is now the longest binary at 0.49–0.75s.
The guard copies 8shu's shape. It counts **parses**, not calls, so it does not
depend on test order. It drives both strategies through a `TestRunner` rather
than re-implementing them. Red before the fix with `left: 64, right: 1`.
Rivals, one per strategy: restoring `let rules = load_grammar();` in either
`formula_string` or `expr_string` alone fails the test with `left: 33`. So the
two strategies are guarded independently.
The case count stays 256. It is already the default that
`context/design-principles.md` §7 prescribes, and it was never the cost.
### export_csv_omits_the_rows_the_view_prunes
improvise-iot6. `render_csv` built its layout with `GridLayout::new`, which does
context/test-evidence/tooling.md
+295
View File
@@ -682,6 +682,78 @@ alone fails, exiting 0. All 44 others pass. That is the review's point: the
older `a head that moves mid-wait is not a pass` starts from MERGEABLE, so it
cannot see this rival.
### The pr-watch red-under-BLOCKED and ended-PR cases — `scripts/pr-watch-test.sh`
improvise-9pfs and improvise-1qaa. Both were found by the re-review of PR #186
at `04230a7`.
**9pfs: the commonest red had no case.** Every red fixture wore `UNSTABLE` or
`UNKNOWN`. Here, though, a failed *required* check reports `MERGEABLE/BLOCKED`,
and the table enumerates green rollups only. The new case is
`a failed required check under MERGEABLE/BLOCKED fails without waiting`. It uses
PR #189's rollup with `Test and lint (host)` rewritten to `FAILURE`, runs at
`--timeout 0`, and expects exit 1. It passed at `202e7eac`, since the script
already broke on any failed check, so it pins existing behaviour. **Rival:** the
failed-check break gated on `state != BLOCKED`. That case alone fails, exiting 3
where 1 is wanted, and the other 45 cases pass. Nothing else in the suite could
see a refactor that turned every required-check red into a timeout.
**1qaa: a merged PR slept the whole timeout.** Two case pairs. Each uses the
shape GitHub returned live, with green checks. MERGED is `UNKNOWN/UNKNOWN`
(#189 once merged). CLOSED is `CONFLICTING/DIRTY` (#74). The CLOSED fixture was
`UNKNOWN/UNKNOWN` at `b13355c7`, and review found that shape unreal. Each pair
asserts exit 5, and that the fixture was read **once**, using the poll counter
`fetch` keeps on disk. Against the script at `202e7eac`:
- MERGED: exit 3 after 102 polls within a 5s timeout.
- CLOSED: exit 1 on the first poll. `CONFLICTING` already ended the wait, but
as "not mergeable".
Two rivals:
- **The state checked only when the timeout expires.** The MERGED read-once case
fails (44 polls), and so does the CLOSED exit-code case, because `CONFLICTING`
breaks to exit 1 before the timeout branch is reached. The poll count is what
pins "does not wait" for MERGED, which is why each case is a pair.
- **Exit 1 instead of 5.** Both exit-code cases fail with exit 1. That keeps
"already merged" apart from "not mergeable".
**29w2: the CLOSED cases could not discriminate.** Found reviewing #253 at
`8cdea3a4`. The only CLOSED shape was #74's `CONFLICTING/DIRTY`, and that ends
the loop on the first poll whether or not the state is checked there. So
**rival R8** passed 52 of 52. R8 exits 5 for MERGED at the loop top, but for
CLOSED only after the loop breaks. Run live against #46 with `--timeout 15`, R8
exited 3 and `8cdea3a4` exited 5. A third shape is added, #46's: CLOSED, green
checks, `MERGEABLE/BLOCKED`, which the loop waits on. #74's shape stays.
- **R8:** both new cases fail, the exit-code case with exit 3 and the read-once
case after 65 polls. The other 52 pass.
- **The state checked only at timeout:** the new read-once case fails (77 polls),
alongside the MERGED read-once case and the `CONFLICTING/DIRTY` exit-code case.
### The pr-watch ended-PR ordering cases — `scripts/pr-watch-test.sh`
improvise-24lz, found reviewing #253 at `b13355c7`. That head checked `state`
after `--expect` but **before** pinning the head. So a watch with no `--expect`
that saw head a running, and then head b MERGED, exited 5 where it should have
exited 2. It accepted a merge at a head the watcher never saw, which is the
PR #67 hazard. Two cases:
- `a head that moves and then merges is not reported as merged`: head a with a
running check, then head b MERGED. Expects 2. At `b13355c7` it exited 5.
- `a PR merged at a head other than --expect is a head mismatch`: head b MERGED,
`--expect aaaa`. Expects 2. It passed at `b13355c7`, where `--expect` already
came first.
The fix moves the `state` check below the pin block. On the first poll that
block only records the head, so an already-merged PR still ends on its first
read. Both new cases pass at `202e7eac`, since it never reads `state`; they pin
the order of the new code. **Rivals:**
- **The state checked before `--expect` and before the pin.** Both new cases fail
with exit 5, and the other 50 pass.
- **The `b13355c7` placement.** Only the moved-then-merged case fails.
## Every JSON parser reads numbers exactly (improvise-dz8c)
### `every_crate_that_can_parse_json_enables_float_roundtrip`
@@ -812,6 +884,63 @@ to corrupt a tracked file and call cargo, which needs the crates.io index, from
`checkers.sh`, whose environment is scrubbed. The by-hand run above is the
evidence.
### The timeout case asserts the exact poll count, and a hang fails — `scripts/pr-reviews-test.sh` (improvise-7fl8)
**What was unguarded.** "--wait keeps polling until its deadline" asserted *at
least* 5 polls for `--wait 5`. On the one-tick-per-read clock the correct loop
polls exactly 6 times: one initial poll, then re-polls until the post-poll clock
read reaches the deadline. The 7epn entry above says the poll count tells a
loop that stopped early from one that reached the deadline. That was true only
for a loop stopping two or more ticks early. Both rivals from the #218 review
passed the whole suite at 25/25, re-run here before the change:
- a loop capped at 5 polls whatever the deadline;
- `deadline = now + WAIT - 1`.
**The fix** asserts `polls = WAIT + 1`, which the fake clock makes exact. It
also catches an over-polling `-le`. Against the new assertion both rivals give
24 passed, 1 failed, each "exit 1 after 5 polls, wanted 1 after 6".
**The hang, which was wider than the ticket said.** 7epn left open that
deleting the deadline check hangs the suite. It does not hang only at the
fake-clock case. It hangs first at "an inline comment at an EARLIER head does
not satisfy --wait", a real-clock `--wait 1` case, and at every other case that
should time out. So the guard wraps *every* call. `"$REVIEWS"` is now a function
that runs the script alongside a `/bin/sleep 20` watchdog and waits for
whichever finishes first (`wait -n -p`, bash 5.1+). If the watchdog wins, it
kills the script and returns 124. `/bin/sleep` is spelled out because the
fake-clock cases put a no-op `sleep` first on `PATH`. A first version that
killed a watchdog subshell left an orphaned `sleep`, and could signal a
recycled pid.
**Rival: the deadline check deleted.** The suite now finishes in 93 s with 21
passed and 4 failed. The four timeout cases fail with exit 124; the fake-clock
one has made about 400 polls by then. Before the guard the same rival never
finished, and an outer `timeout 30` killed it at exit 124.
### The bash floor is 5.1 — `scripts/checkers-test.sh` (improvise-fd0j)
**What broke.** `pr-reviews-test.sh` calls `wait -n -p`, and `-p` arrived in
bash 5.1. `checkers.sh`'s `find_modern_bash` accepted any major version 5. So a
bash 5.0 on `PATH` was pinned, and that suite then failed on an invalid option
rather than on anything it tests.
**Fix.** The probe reads major and minor in one call and requires
`major*100 + minor >= 501`. The exit-3 message and the header say 5.1.
`context/code/tooling.md` states why the floor is 5.1.
**Cases** (section 5b). A fake bash answering `5 0` with no nix → exit 3. The
control, a fake answering `5 1` → exit 0, proves that refusal comes from the
version and not from the fake.
**Rivals**, run against the real suite in a copied sandbox:
- **Floor 5.0** (`-ge 500`) fails "a bash 5.0 with no nix is exit 3, not 5.x"
and nothing else.
- **The pre-fix runner** fails only the 5.1 control. Its probe asked for the
major version alone, so the fake's `5 1` was not a number and was refused.
The 5.0 case passes against it for the same reason, so that case alone would
not have distinguished the old code. The 5.0-floor rival is the one that
does.
### The dead-ID audit compares positively — `scripts/bd-audit-ids-test.sh` (improvise-52eq)
**What broke.** The audit recipe in `AGENTS.md` ended
@@ -847,3 +976,169 @@ So the pass needs both the exclusions and git's file list.
After each rival the original was restored (`cmp` against a saved copy) and the
suite re-run at 8 passed.
### The audit tells grep's no-match from its read error — `scripts/bd-audit-ids-test.sh` (improvise-lqs0)
**What broke.** `bd-audit-ids` treated xargs exit 123 as "a grep batch matched
nothing". xargs also exits 123 when grep exits 2, so a cited file the scan could
not read passed the audit, and any dead id inside it was missed. The usage text
promised exit 4 for that. Two smaller mismatches with the stated contract:
- run outside a git repository, the audit exited 1, which is the "dead id" code;
- given a bd list whose objects carry no `id`, it reported every cited id dead
(exit 1) where "bd did not answer" is 3.
**Reproduced before the fix, with six new cases:** 10 passed, 4 failed.
- a dangling symlink among the cited files: exit 1, the read error ignored;
- an unreadable (`chmod 000`) file holding the only dead id: **exit 0, "all
resolve"**, which is the false pass;
- outside a repository: exit 1;
- id-less JSON: exit 1.
The control (both files readable, dead id found: exit 1) and the deleted-file
case passed before and after.
**The fix:**
- Each grep batch runs through `bash -c` and maps exit 1 to success and exit 2
to 255. xargs stops on 255 with 124, so a read error is exit 4.
- A tracked file deleted from the working tree is filtered out **on purpose**.
Its ids are leaving the tree, which is the case the ticket asked to decide
rather than leave to accident. A dangling symlink is still an entry, so it
still counts as a read error.
- Outside a repository is exit 4.
- bd's output must be a JSON array whose every element has a string `id`, or
the audit is exit 3.
**Rivals, each run against the suite (the script restored and checked with
`cmp` after each):**
- **read error mapped to success** (`-le 2`): fails both read-error cases, 12
passed and 2 failed;
- **the deleted-file filter removed**: fails "a tracked file deleted from the
working tree is not scanned" with exit 4. So the decision is pinned, not
incidental;
- **the old `cd "$(git rev-parse --show-toplevel)"`**: fails "run outside a git
repository is exit 4" with exit 1. A first attempt at this rival still exited
4 and proved nothing; it was replaced;
- **the id check reduced to `type == "array"`**: fails the id-less case with
exit 1.
The `chmod 000` case is skipped when the suite runs as root, where the file
stays readable. The dangling-symlink case covers the same path for every user.
### Absent is not deleted — `scripts/bd-audit-ids-test.sh` (improvise-tdfd)
**What broke.** The file filter `[ -e "$f" ] || [ -L "$f" ]` dropped every
tracked path missing from the tree, and took each one to be a deletion. A path
marked `--skip-worktree` (or left out by a sparse checkout) is absent too, but
it is not deleted. Its ids were never read, and the audit passed. ChatGPT
Codex found this on PR #244. The adversarial review reproduced it with
`context/adr/001-concurrent-edits.md`, the only citation of one id: exit 1 → 0.
**Fix, Codex's suggestion.** An absent path is skipped only when
`git ls-files -d` lists it. Any other absent tracked path is exit 4, and the
path is named. The xargs comment no longer claims 124. That is GNU; BSD exits
1. The script tests nonzero, so both work.
**Red first.** "a skip-worktree file missing from the tree is exit 4, and
named" exited 0 against the pre-fix script, reporting "all 1 cited ids
resolve".
**Rival:** every absent path reported as unread (the `ls-files -d` test
removed). It fails "a tracked file deleted from the working tree is not
scanned" alone, so the deleted-file case is what stops the fix from
over-refusing. The rival ran from an ignored copy inside the worktree. A copy
under `/tmp` fails "the real tree does not pass against a silent bd" for a
reason unrelated to the rival: there is no repository there.
### Section 5b runs on a hermetic PATH — `scripts/checkers-test.sh`
**What broke.** Every PR's "Knowledge base consistency" job went red at
`666de199` with "a bash 5.0 with no nix is exit 3, not 5.x (exit 1, wanted 3)"
and "a bash 5.1 is accepted (exit 1, wanted 0)". Neither PR alone was wrong. The
NixOS-runner change (`af44653f`) found that `/usr/bin:/bin` on NixOS holds only
`env` and `sh`, so the runner dies on `dirname` with exit 1. It fixed section 5
by moving it to a `utilbin` of linked tools. #237 (`e45a9c42`) added section 5b
on its own branch, with the old `PATH=$work/fakebin:/usr/bin:/bin`. The two
merged with no conflict, and each had passed CI only on its own side.
**Reproduced** on macOS by running both 5b invocations with `/usr/bin:/bin`
replaced by a directory holding only `env` and `sh`. Both exited 1 on
`dirname: command not found`. Those are the CI exits.
**Fix.** 5b uses `utilbin` as well. `utilbin` gains `head` and `env`, because
the 5.1 control runs past the version check into `check_shebangs` and
`run_suite`.
**Rivals**, each run in the suite:
- **`dirname` left out of `utilbin`** fails all three section-5 cases with the
exact CI pair: exit 1 where 3 was wanted, and exit 1 where 0 was wanted. The
NixOS failure now reproduces on any host, because no case reads the host's
`/usr/bin` any more.
- **`env` left out** fails "a bash 5.1 is accepted" alone.
- **`head` left out** fails nothing. `check_shebangs` calls `head` inside
`check_shebangs || rc=1`, where `set -e` is off. A missing `head` makes every
first line empty, and the shebang check then passes without checking
anything. That is a defect in `checkers.sh` and is not fixed here.
### The pr-reviews watchdog resolves its tools from PATH — `scripts/pr-reviews-test.sh` (improvise-3uow)
**What broke.** The suite hard-coded `/bin/sleep` for the watchdog in
`bounded_reviews` and `/bin/date` for the fake clock's pass-through. On the
NixOS runners `/bin` holds only `sh`. With no `/bin/sleep`, the watchdog exits
at once. `wait -n` then sees it finish before `pr-reviews`, and every bounded
call reports 124 ("killed after 20s"). CI never showed this, because the kb job
stopped one step earlier on the section-5b failure (#296).
**Reproduced** by pointing both paths at `/nonexistent/`. The result was 0
passed and 25 failed, every `--wait` case exiting 124: the same shape the
reviewer simulated.
**Fix.** `REAL_SLEEP` comes from `type -P`, before any case puts its stub
`sleep` first on PATH. `type -P` returns a path even if `sleep` has been loaded
as a builtin. The fake clock's `/bin/date` pass-through was deleted rather than
re-pointed. `pr-reviews` only ever calls `date +%s`, so the branch never ran
(found by Copilot on #297). The fake now refuses any other arguments with
exit 2, so a new `date` call in `pr-reviews` shows up as a failure instead of
untested code.
**Rivals:**
- **`REAL_SLEEP=/nonexistent/sleep`** fails the whole suite, 0 of 25, so the
resolved path is the one the watchdog runs.
- **`pr-reviews` changed to call `date -u +%s`** fails "a review arriving on
the third poll is seen" and "--wait keeps polling until its deadline" (23 of
25 pass). The fake's refusal is therefore visible, not swallowed.
**Scope sweep.** `grep -rnE '/(usr/)?bin/[a-z]'` over `scripts/`, `web/*.sh`,
`harness/*.sh`, `harness/tests` and `.github/workflows` finds only `#!/bin/sh`
shebangs, and `/bin/sh` exists on NixOS. It also found a
`${PATH:-/usr/bin:/bin}` fallback in `checkers.sh`, which is now removed (next
entry).
### The bash floor and its self-tests are retired — `scripts/checkers.sh`
**The ruling.** The user ruled that CI should not care which version of bash or
of the standard utilities it runs, and that CI jobs should use whatever the
`.#ci` dev shell provides. So `checkers.sh` no longer pins a bash:
`find_modern_bash`, the 5.1 floor (improvise-fd0j) and exit 3 are gone. Suites
run under `$BASH`, the bash running the runner, with `PATH` unchanged. The
reasoning is in `context/code/tooling.md` §5.
**Tests removed.** Three sections of `checkers-test.sh` were removed. They
existed only to pin the floor, and the entries above that cite them describe
cases that no longer exist:
- Section 2, "a bash-3.2-only construct fails under the runner's pinned bash".
It asserted a bash-5 behaviour, so its result depended on the bash version.
- Section 5, "an old bash with no nix is exit 3".
- Section 5b, "a bash 5.0 with no nix is exit 3, not 5.x" and "a bash 5.1 is
accepted", together with the `utilbin` that #296 added for them.
The suite keeps 10 cases: environment scrubbing (with its direct-run pair), the
shebang rule (with its control), fail and pass, unknown suite, and `--list`.
**Kept deliberately.** The environment allowlist. That is what the
`set -u`/`$name` evidence above earns, and it does not depend on any version.
**Also removed with the ruling.** `scripts/kb`'s `stat -f` fallback: `kb`
already requires the dev shell for `tpl`, and the dev shell's `stat` is GNU.
From `ci.yml`, the toolchain version probes ("the dev shell supplies the pinned
web toolchain", "Playwright matches the dev shell's browsers") and every
`shell: bash` override, so every step runs in `nix develop .#ci`.
context/test-evidence/tui.md
+57
View File
@@ -1204,3 +1204,60 @@ than `App::handle_key`, because the expiry is the front end's rule.
at step 1: `" TILES Saved to x.improv …"`.
- With the expiry moved *after* dispatch, it fails at step 2: "the axis cycle
must say what it did", since every message is wiped before it can be drawn.
### a_minibuffer_refusal_stands_while_the_user_fixes_it
improvise-su4u. It drives `draw.rs::press`, the function `run_tui` calls,
because the expiry is the TUI loop's rule. In the formula panel it presses `n`,
types `Total = (` and `Enter`. The formula does not parse, so the editor stays
open with `Formula error` on the bottom row. Then:
1. `Backspace`: still editing, and the error must still be drawn. It failed here
at base 202e7eac: `"formula: Total = ▌"`, with the error gone.
2. `Enter` on the still-bad `Total = `: a new refusal, which must differ from
the first, and whose text must be on the row. A key's own message beats the
stale one.
3. `1` and `Enter`: the fixed formula commits and leaves the editor, and the
error must be gone.
4. A second run: `Esc` straight after the refusal must drop the error too.
Rivals, each injected into `press` and restored:
- **Never restore** (#233's behaviour) fails step 1.
- **Restore even when the key wrote its own message** fails step 2.
- **Restore whatever the mode after the key** fails step 4, because the error
outlives `Esc`. It does not fail step 3, because a successful commit writes
its own message. That is why step 4 exists.
### a_message_does_not_follow_the_editor_onto_the_next_cell
improvise-sx7h, a regression #257 introduced against #233. Equal modes stood for
"the same editor", but `Editing`/`RecordsEditing` commit by re-opening an equal
mode on the next cell. The test drives `press` through four scenarios:
1. **Records.** `R`, then `o` ("Added new record row", in `RecordsEditing`),
then `5`. The message stands while typing in the same editor. Then `Tab`: the
editor re-opens on the next cell, and the message must be gone.
2. **Records.** `R`, `o`, `Esc`, then `o` again, which is refused with "That
record row is already here…". Then `5`, with the refusal still drawn. Then
`Tab`, which fills the row in, and the refusal must be gone.
3. **A plain grid.** `i` on `(0, 0)`, with a note set while editing. Then `7`,
with the note kept. Then `Enter`: the editor re-opens a row down, and the
note must be gone.
4. **The last column.** `i`, a note, `7`, `Tab`. The cursor cannot advance, so
the editor re-opens on the **same** cell, and the note must be gone.
Scenarios 1 and 2 are the reviewer's repro.
Rivals, each injected into `EditorSession::of` and restored:
- **mode only**, which is #257's rule as pushed at 2b1c5bf7, fails at
scenario 1;
- **no cursor cell** fails at scenario 1, because the new record's cells both
seed `""`, so the seed alone cannot tell them apart;
- **no seed** fails at scenario 4, the only place the cell does not change.
### an_editor_on_another_category_is_another_editor
improvise-sx7h's pinning test, asked for by the review. The mode is compared
whole, not by variant. Two `ItemAdd` sessions for different categories must be
different editors, and the same category must be the same. No key reaches that
transition, so it is asserted on `EditorSession::of` directly. The rival that
compares `std::mem::discriminant` fails it, and passes every other test.
crates/improvise-command/src/command/cmd/cell.rs
+48
View File
@@ -525,6 +525,54 @@ mod tests {
);
}
/// The commit door alone on a formula's cell, with no advance to reach it by.
/// See context/test-evidence/command.md.
#[test]
fn an_editor_already_open_on_a_formula_cell_commits_nothing_under_it() {
let mut app = formula_app();
app.cursor_mut().selected = (1, 2);
app.view_state.mode = crate::ui::app::AppMode::editing();
app.view_state.buffers.insert("edit".into(), "7".into());
app.handle_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE))
.unwrap();
let (text, back) = saved_and_reloaded(&app.model_state.workbook);
assert!(
!text.contains("_Measure=Profit"),
"saved under the formula:\n{text}"
);
assert_eq!(back.model.get_cell(&record("West", "Profit")), None);
}
/// improvise-enz6's formula twin: Tab does not re-open the editor on a cell `i`
/// would decline. See context/test-evidence/command.md.
#[test]
fn tab_advancing_onto_a_formula_cell_does_not_reopen_the_editor_there() {
let tab = KeyEvent::new(KeyCode::Tab, KeyModifiers::NONE);
let mut app = formula_app();
app.cursor_mut().selected = (1, 0);
press(&mut app, 'i');
app.handle_key(tab).unwrap();
assert_eq!(app.cursor().selected, (1, 1), "setup: Tab moved to Cost");
assert!(
app.view_state.mode.is_editing(),
"the control: Cost is stored, so the editor re-opens there"
);
app.handle_key(tab).unwrap();
assert_eq!(app.cursor().selected, (1, 2), "Tab still moves onto Profit");
assert!(
!app.view_state.mode.is_editing(),
"an editor was opened on the formula: {:?}",
app.view_state.mode
);
assert!(
app.view_state
.status_msg
.contains("Profit is computed by a formula"),
"the decline is out loud, got {:?}",
app.view_state.status_msg
);
}
/// The second door onto a formula cell: Tab advances the editor onto it, where
/// `i` would have declined. See context/test-evidence/command.md.
#[test]
crates/improvise-command/src/command/cmd/commit.rs
+72 -1
View File
@@ -7,7 +7,7 @@ use crate::ui::effect::{self, Effect};
use crate::view::RecordCol;
use improvise_protocol::{OpenedEditor, untouched};
use super::core::{Cmd, CmdContext, read_buffer};
use super::core::{Cmd, CmdContext, read_buffer, write_buffer};
use super::grid::AddRecordRow;
use super::navigation::{CursorState, EnterAdvance, viewport_effects};
@@ -830,6 +830,72 @@ mod tests {
"the reopened editor did not capture the cell it landed on"
);
}
/// Every cell of the 2×2 fixture, by its key.
fn every_cell(app: &crate::ui::app::App) -> Vec<Option<CellValue>> {
use crate::model::cell::CellKey;
let mut cells = Vec::new();
for region in ["North", "West"] {
for measure in ["Cost", "Revenue"] {
let key = CellKey::new(vec![
("Region".into(), region.into()),
("_Measure".into(), measure.into()),
]);
cells.push(app.model_state.workbook.model.get_cell(&key).cloned());
}
}
cells
}
/// BUG (improvise-fy9o) — see `context/test-evidence/command.md`.
#[test]
fn enter_enter_leaves_every_cell_unchanged() {
use improvise_protocol::key::KeyCode;
for (records, start) in [(false, (1, 0)), (true, (1, 2))] {
let mut app = grid_holding(CellValue::Number(4.0), records);
if records {
app.view_state.mode = AppMode::RecordsNormal;
}
app.cursor_mut().selected = start;
let before = every_cell(&app);
press(&mut app, KeyCode::Char('i'));
press(&mut app, KeyCode::Enter);
let reopened = app.view_state.buffers.get("edit").cloned();
press(&mut app, KeyCode::Enter);
assert_eq!(
every_cell(&app),
before,
"records={records}: i, Enter, Enter changed a cell (the reopened editor held {reopened:?})"
);
}
}
/// The reopened editor shows the cell it landed on, so typing extends it.
/// BUG (improvise-fy9o) — see `context/test-evidence/command.md`.
#[test]
fn typing_after_the_reopen_still_writes() {
use crate::model::cell::CellKey;
use improvise_protocol::key::KeyCode;
let mut app = grid_holding(CellValue::Number(4.0), false);
app.cursor_mut().selected = (1, 0);
press(&mut app, KeyCode::Char('i'));
press(&mut app, KeyCode::Enter);
assert_eq!(
app.view_state.buffers.get("edit").map(String::as_str),
Some("2"),
"the editor reopened on (North, Revenue) is not seeded with its value"
);
press(&mut app, KeyCode::Char('5'));
press(&mut app, KeyCode::Enter);
let landed = CellKey::new(vec![
("Region".into(), "North".into()),
("_Measure".into(), "Revenue".into()),
]);
assert_eq!(
app.model_state.workbook.model.get_cell(&landed),
Some(&CellValue::Number(25.0))
);
}
}
// ── Commit commands (mode-specific buffer consumers) ────────────────────────
@@ -976,6 +1042,11 @@ impl Cmd for CommitAndAdvance {
// Drop to normal **before** the commit, which can abort: tidy-up goes where
// an abort cannot reach it (improvise-2zq, gotcha 21).
effects.push(effect::change_mode(exit_mode_for(&self.edit_mode)));
// This command owns its buffer's clear, at the head, so the re-entry
// below re-seeds after it rather than being blanked (improvise-fy9o).
if let Some(mb) = self.edit_mode.minibuffer() {
effects.push(write_buffer(&mb.buffer_key, String::new()));
}
// An untouched editor writes nothing; everything around the write still runs.
let writes = !untouched(self.opened.as_ref(), &self.value);
if writes {
crates/improvise-command/src/command/cmd/core.rs
+20 -6
View File
@@ -27,6 +27,22 @@ pub fn derived_refusal(target: &str) -> String {
format!("{target} is computed by a formula — edit the formula, not the cell")
}
/// Why no editor may open on or commit to `(row, col)`: an aggregate (a fold
/// named as one, §14) or a formula's result. `i` declines the same two arms.
pub fn uneditable(model: &Model, layout: &GridLayout, row: usize, col: usize) -> Option<String> {
match crate::ui::project::cell_id(model, layout, row, col) {
Some(CellId::Aggregate(_)) => Some(
match layout.folds_a_group(row, col) {
true => FOLD_REFUSAL,
false => AGGREGATE_REFUSAL,
}
.to_string(),
),
Some(CellId::Derived { target }) => Some(derived_refusal(&target)),
Some(CellId::Record(_) | CellId::Create(_) | CellId::Coordinate) | None => None,
}
}
/// Read-only context available to commands for decision-making: a `&Model`, the
/// active `&View`, and the `&Workbook` for the rare command that enumerates views.
pub struct CmdContext<'a> {
@@ -99,12 +115,10 @@ impl<'a> CmdContext<'a> {
/// The cursor's key for a commit, which resolves a records position itself
/// (gotcha 19). An aggregate or a formula's result is refused, as `i` refuses it.
pub fn committable_cell_key(&self) -> Result<CellKey, String> {
match self.cursor_cell_id() {
Some(CellId::Aggregate(_)) => Err(self.aggregate_refusal()),
Some(CellId::Derived { target }) => Err(derived_refusal(&target)),
Some(CellId::Record(_) | CellId::Create(_) | CellId::Coordinate) | None => {
self.cell_key().ok_or_else(|| NO_CELL.to_string())
}
let (row, col) = self.selected;
match uneditable(self.model, self.layout, row, col) {
Some(refusal) => Err(refusal),
None => self.cell_key().ok_or_else(|| NO_CELL.to_string()),
}
}
crates/improvise-command/src/command/cmd/grid.rs
+57
View File
@@ -66,6 +66,63 @@ mod tests {
assert_eq!(non_item_coordinates(&back), Vec::<(String, String)>::new());
}
/// improvise-enz6, the entry half: the editor that Enter re-opens is not opened
/// on a fold. See context/test-evidence/command.md.
#[test]
fn enter_advancing_onto_a_fold_does_not_reopen_the_editor_there() {
use crate::command::key::{KeyCode, KeyEvent, KeyModifiers};
let enter = KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE);
let mut app = app_on_a_fold();
app.cursor_mut().selected = (0, 1);
press(&mut app, 'i');
app.handle_key(enter).unwrap();
assert_eq!(app.cursor().selected, (1, 1), "setup: Enter moved to Feb");
assert!(
app.view_state.mode.is_editing(),
"the control: Feb is an item, so the editor re-opens there"
);
app.handle_key(enter).unwrap();
assert_eq!(
app.cursor().selected,
(2, 1),
"Enter still moves onto the fold"
);
assert!(
!app.view_state.mode.is_editing(),
"an editor was opened on the fold: {:?}",
app.view_state.mode
);
assert!(
app.view_state.status_msg.contains("ollapsed group"),
"the decline is out loud, got: {:?}",
app.view_state.status_msg
);
}
/// The commit door alone, now the advance no longer opens an editor on a fold.
/// See context/test-evidence/command.md.
#[test]
fn an_editor_already_open_on_a_fold_commits_nothing_there() {
use crate::command::key::{KeyCode, KeyEvent, KeyModifiers};
let mut app = app_on_a_fold();
app.view_state.mode = AppMode::editing();
app.view_state.buffers.insert("edit".into(), "7".into());
app.handle_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE))
.unwrap();
let (text, back) = saved_and_reloaded(&app.model_state.workbook);
let data = text.split("## Data").nth(1).expect("a data section");
assert!(
!data.contains("Q2"),
"a group coordinate was saved:\n{text}"
);
assert_eq!(non_item_coordinates(&back), Vec::<(String, String)>::new());
assert!(
app.view_state.status_msg.contains("ollapsed group"),
"the refusal is out loud, got: {:?}",
app.view_state.status_msg
);
}
/// The control for the refusal above: the same key one row up drills.
#[test]
fn drilling_into_the_row_above_a_fold_still_opens_a_drill() {
crates/improvise-command/src/command/keymap.rs
+36 -9
View File
@@ -753,6 +753,22 @@ mod tests {
}
/// The buffer `append-char` is bound to in `mode`'s keymap.
/// The effects `name` builds in `mode` with `buffer` holding text, as Debug.
fn clears_itself(mode: &AppMode, name: &str, args: &[String], buffer: &str) -> String {
use crate::command::cmd::test_helpers::*;
let wb = two_cat_model();
let layout = make_layout(&wb);
let reg = make_registry();
let bufs: HashMap<String, String> = [(buffer.to_string(), "typed".to_string())].into();
let mut ctx = make_ctx(&wb, &layout, &reg);
ctx.mode = mode;
ctx.buffers = &bufs;
let cmd = reg
.interactive(name, args, &ctx)
.unwrap_or_else(|e| panic!("{name} {args:?} does not build: {e}"));
effects_debug(&cmd.execute(&ctx))
}
fn append_target(ks: &KeymapSet, mode: &AppMode) -> String {
let key = ModeKey::from_app_mode(mode, false).expect("a text-entry mode key");
let km = ks.maps().get(&key).expect("every mode has a keymap");
@@ -827,15 +843,26 @@ mod tests {
let km = ks.maps().get(&mode).unwrap();
for key in exits {
let Some(Binding::Sequence(steps)) = km.lookup(*key, none) else {
panic!("{mode:?}: {key:?} should be a commit/exit sequence");
};
assert!(
steps
.iter()
.any(|(n, a)| *n == "clear-buffer" && a.as_slice() == [buffer.to_string()]),
"{mode:?}: {key:?} must clear \"{buffer}\", got {steps:?}"
);
match km.lookup(*key, none) {
Some(Binding::Sequence(steps)) => {
assert!(
steps.iter().any(|(n, a)| *n == "clear-buffer"
&& a.as_slice() == [buffer.to_string()]),
"{mode:?}: {key:?} must clear \"{buffer}\", got {steps:?}"
)
}
// A reopening commit owns its clear (improvise-fy9o): ask the command.
Some(Binding::Cmd { name, args }) => {
let dbg = clears_itself(&app_mode, name, args, &buffer);
assert!(
dbg.contains(&format!(
"SetBuffer {{ name: \"{buffer}\", value: \"\" }}"
)),
"{mode:?}: {key:?} runs {name} alone, which must clear \"{buffer}\": {dbg}"
);
}
other => panic!("{mode:?}: {key:?} should commit or exit, got {other:?}"),
}
}
}
}
crates/improvise-command/src/ui/app.rs
+49 -8
View File
@@ -587,19 +587,17 @@ impl App {
f.target
)
});
let refusal = self.group_refusal(&key).or(derived);
let refusal = self.non_item_refusal(&key).or(derived);
refusal.or_else(|| self.aggregate_refusal(cell.record, &key))
}
/// A coordinate naming a group rather than an item is no cell in ANY view, so
/// this asks the document, never the layout (improvise-b114).
fn group_refusal(&self, key: &CellKey) -> Option<String> {
/// Every coordinate must name an item of its category, asked of the document
/// (improvise-b114, improvise-nacd). See `context/code/command.md`.
fn non_item_refusal(&self, key: &CellKey) -> Option<String> {
let model = &self.model_state.workbook.model;
key.0.iter().find_map(|(cat, name)| {
let category = model.category(cat)?;
let group = !category.items.contains_key(name.as_str())
&& !category.items_in_group(name).is_empty();
group.then(|| format!("{name} is a group of {cat}, so there is no one cell to write"))
let item = model.effective_item_names(cat).contains(name);
(!item).then(|| format!("{name} is no item of {cat}, so there is no one cell to write"))
})
}
@@ -998,6 +996,10 @@ impl App {
if self.abort_effects {
// An abort-setting effect asked for the rest to be skipped. Reset
// the flag, and record the outcome where a caller can read it.
debug_assert!(
matches!(kind, EffectKind::ViewOnly) || self.batch_refused,
"a model effect aborted without refusing: {effect:?} (improvise-lasd)"
);
self.abort_effects = false;
self.batch_aborted = true;
break;
@@ -3298,6 +3300,21 @@ mod tests {
app
}
/// Storage holding one loadable document, `doc.improv`, and nothing else.
fn with_a_document_to_load() -> App {
let storage = crate::storage::MemoryFs::new();
let text = crate::persistence::format_md(&Workbook::new("Doc"));
crate::storage::Storage::write(
&storage,
std::path::Path::new("doc.improv"),
text.as_bytes(),
)
.expect("fixture writes");
let mut app = two_col_model();
app.storage = Box::new(storage);
app
}
/// Months grouped into quarters with Q2 collapsed: row 2 is the fold.
fn folded_quarter() -> App {
let mut wb = Workbook::new("T");
@@ -3521,6 +3538,30 @@ mod tests {
run_line(app, "search-navigate forward")
},
),
(
"load of an absent file",
with_a_document_to_load,
|app| run_line(app, "load absent.improv"),
"Load error",
with_a_document_to_load,
|app| run_line(app, "load doc.improv"),
),
(
"import-json of an absent JSON",
|| with_import_fixtures(two_col_model()),
|app| run_line(app, "import-json absent.json"),
"Cannot read 'absent.json'",
|| with_import_fixtures(two_col_model()),
|app| run_line(app, "import-json good.json"),
),
(
"a formula that does not parse",
two_col_model,
|app| commit_in(app, AppMode::formula_edit(), "formula", "Total = ("),
"Formula error",
two_col_model,
|app| commit_in(app, AppMode::formula_edit(), "formula", "Total = 1"),
),
(
"a : line that does not parse",
two_col_model,
crates/improvise-command/src/ui/effect.rs
+14 -3
View File
@@ -324,6 +324,20 @@ pub struct EnterEditAtCursor {
}
impl Effect for EnterEditAtCursor {
fn apply(&self, app: &mut App) {
let (row, col) = app.cursor().selected;
let model = &app.model_state.workbook.model;
match crate::command::cmd::core::uneditable(model, &app.layout, row, col) {
Some(refusal) => app.view_state.status_msg = refusal,
None => self.open(app),
}
}
fn kind(&self) -> EffectKind {
EffectKind::ViewOnly
}
}
impl EnterEditAtCursor {
/// Open the editor on the cursor's cell, seeded with its unformatted value.
fn open(&self, app: &mut App) {
let value = app.display_value_at_cursor();
app.view_state
.buffers
@@ -331,9 +345,6 @@ impl Effect for EnterEditAtCursor {
app.view_state.opened_editor = Some(improvise_protocol::OpenedEditor::on(value));
app.view_state.mode = self.target_mode.clone();
}
fn kind(&self) -> EffectKind {
EffectKind::ViewOnly
}
}
#[derive(Debug)]
crates/improvise-command/src/ui/host.rs
+173
View File
@@ -1468,6 +1468,20 @@ mod tests {
}
}
/// improvise-nacd: a formula target is a measure, so the refusal names the
/// formula. Evidence: context/test-evidence/command.md.
#[test]
fn a_write_under_a_formula_is_refused_for_the_formula_not_as_a_non_item() {
let host = host_on_a_fold();
let key = month_cell("Feb", "Profit");
let cell = RecordCell {
record: RecordHash::of(&key),
measure: Some("Profit".into()),
};
let why = host.app().unwritable(&cell).expect("refused");
assert!(why.contains("formula"), "refused for another reason: {why}");
}
/// improvise-ciw8: a formula's result is `CellId::Derived`, no write's target.
#[test]
fn a_wire_write_under_a_formula_is_rejected_and_stores_nothing() {
@@ -1639,6 +1653,59 @@ mod tests {
}
}
/// `Month` Jan/Feb, `_Measure` Revenue and Cost in `Money` under the parent
/// group `Totals`, and the record `{Month=Feb}` stored.
fn host_on_a_stored_record_beside_groups() -> Host {
let mut wb = Workbook::new("T");
wb.add_category("Month").unwrap();
for month in ["Jan", "Feb"] {
wb.add_item("Month", month).unwrap();
}
for measure in ["Revenue", "Cost"] {
wb.add_item_in_group("_Measure", measure, "Money").unwrap();
}
let measures = wb.model.category_mut("_Measure").unwrap();
measures.add_group(crate::model::category::Group::new("Money").with_parent("Totals"));
wb.model
.set_cell(month_cell("Feb", "Revenue"), CellValue::Number(10.0));
wb.active_view_mut().set_axis("Month", Axis::Row);
wb.active_view_mut().set_axis("_Measure", Axis::Column);
let mut app = App::new(wb, None);
app.rebuild_layout();
Host::new(app)
}
/// improvise-pr8t: an item that shares its name with a group of its category
/// is still an item. Evidence: context/test-evidence/command.md.
#[test]
fn a_wire_write_to_an_item_named_like_its_categorys_group_is_accepted() {
let mut host = host_on_a_stored_record_beside_groups();
let measures = host.app.model_state.workbook.model.category_mut("_Measure");
measures.unwrap().add_item("Money");
host.app.rebuild_layout();
let target = month_cell("Feb", "Money");
let answer = wire_write(&mut host, &target, false);
let accepted = answer
.iter()
.any(|a| matches!(a.msg, ServerFrame::Accepted));
assert!(accepted, "a real item was refused: {answer:?}");
let model = &host.app().model_state.workbook.model;
assert_eq!(model.get_cell(&target), Some(&CellValue::Number(7.0)));
}
/// improvise-nacd: a stored record's measure must name an item, not merely
/// avoid naming a group. Evidence: context/test-evidence/command.md.
#[test]
fn a_wire_write_naming_no_item_of_its_category_is_rejected() {
for measure in ["Bogus", "Totals"] {
let mut host = host_on_a_stored_record_beside_groups();
let before = document_text(&host);
let answer = wire_write(&mut host, &month_cell("Feb", measure), false);
assert!(refused(&answer), "{measure} accepted: {answer:?}");
assert_eq!(document_text(&host), before, "{measure} was stored");
}
}
/// improvise-xur5 (the reviewer's `probe_fold_hidden_measure`): a fold's record
/// with a measure the host draws nowhere — see context/test-evidence/command.md.
#[test]
@@ -2386,6 +2453,112 @@ mod tests {
}
}
/// improvise-we1a: `mutating_frame`'s `|| last_batch_aborted()` arm is redundant
/// only while this holds. Evidence: `context/test-evidence/command.md`.
#[test]
fn every_failing_model_effect_that_aborts_also_refuses() {
let fresh = || {
let mut wb = workbook();
wb.create_view("Scratch");
(0..12).for_each(|n| drop(wb.add_category(format!("Filler{n}"))));
let mut app = App::new(wb, None);
app.storage = Box::new(super::import_fixture_storage());
app.rebuild_layout();
app
};
let cases: Vec<(&str, ModelCommand, bool)> = vec![
(
"AddCategory past the limit",
ModelCommand::AddCategory("Extra".into()),
true,
),
(
"AddFormula that does not parse",
ModelCommand::AddFormula {
raw: "Total = (".into(),
target_category: "_Measure".into(),
},
true,
),
(
"LoadModel of an absent file",
ModelCommand::LoadModel(PathBuf::from("absent.improv")),
true,
),
(
"import of an absent JSON",
import("absent.json", None),
true,
),
("import of an absent CSV", import("absent.csv", None), true),
(
"import of a CSV that is not text",
import("bad.csv", None),
true,
),
(
"import of JSON that does not parse",
import("bad.json", None),
true,
),
(
"import at a path with no array",
import("nested.json", Some("nope")),
true,
),
(
"import that builds no model",
import("measures.json", None),
true,
),
(
"AddItem into an absent category",
ModelCommand::AddItem {
category: "Ghost".into(),
item: "East".into(),
},
false,
),
(
"AddItemInGroup into an absent category",
ModelCommand::AddItemInGroup {
category: "Ghost".into(),
item: "East".into(),
group: "Coastal".into(),
},
false,
),
(
"AddFormula into an absent category",
ModelCommand::AddFormula {
raw: "Total = 1".into(),
target_category: "Ghost".into(),
},
false,
),
(
"DeleteView of an absent view",
ModelCommand::DeleteView("Ghost".into()),
false,
),
];
for (what, cmd, aborts) in cases {
let mut app = fresh();
let effects = crate::ui::reduce::model_effects(&app, &cmd);
app.apply_effects(effects);
assert_eq!(
app.last_batch_aborted(),
aborts,
"{what}: which failures abort is part of what this table pins"
);
assert!(
app.last_batch_refused(),
"{what}: failed without refusing, so a frame carrying it is rejected \
only by mutating_frame's `|| last_batch_aborted()` arm"
);
}
}
/// improvise-9fpl: one case per refusal #181 converted that no other test
/// witnesses. Evidence: `context/test-evidence/command.md`.
#[test]
crates/improvise-command/src/ui/project.rs
+274 -4
View File
@@ -256,11 +256,22 @@ fn column_totals(body: &GridBody, cols: std::ops::Range<usize>) -> Vec<CellDispl
true => Vec::new(),
false => cols
.filter_map(|c| Some((body.totals.get(c)?, body.col_formats.get(c)?)))
.map(|(total, fmt)| CellDisplay::number(fmt.number(*total)))
.map(|(total, fmt)| total_display(total.as_ref(), fmt))
.collect(),
}
}
/// One total as the strip draws it: a number, a unanimous text, an error, or
/// nothing where the column holds no record (improvise-wm20).
fn total_display(total: Option<&CellValue>, fmt: &crate::format::NumberFormat) -> CellDisplay {
match total {
Some(CellValue::Number(n)) => CellDisplay::number(fmt.number(*n)),
Some(v @ CellValue::Error(_)) => CellDisplay::error(fmt.value(Some(v))),
Some(v @ CellValue::Text(_)) => CellDisplay::text(fmt.value(Some(v))),
None => CellDisplay::default(),
}
}
/// One cell as the client draws it. The server knows whether a value was a number
/// and whether the cell is settable, so it says both (ADR 002, [`cell_id`]).
fn cell_display(
@@ -2951,7 +2962,7 @@ mod tests {
);
assert_eq!(
one_row.totals,
vec![1.0],
vec![Some(CellValue::Number(1.0))],
"a column total sums every row, including the ones scrolled past"
);
assert_eq!(
@@ -4854,8 +4865,8 @@ mod cell_identity {
// The totals strip is formatted on its own path (`col_formats`), so it
// can disagree with the cells above it without any cell being wrong.
assert_eq!(
grid.totals[margin].text, "1.5000",
"three rows of 0.5, under the column's own format"
grid.totals[margin].text, "0.5000",
"the formula over the pooled rows (3 / 6), under the column's own format"
);
assert_eq!(grid.totals[revenue].text, "3.00");
}
@@ -5820,3 +5831,262 @@ mod basis_check {
assert_eq!(value_at(&app, 1), "99.00");
}
}
/// The totals strip: a column's total is its cell as if every drawn row were one
/// fold (improvise-wm20). The rule and the rulings: `context/code/command.md`.
#[cfg(test)]
mod column_totals {
use super::*;
use crate::model::cell::{CellKey, CellValue};
use crate::workbook::Workbook;
use improvise_core::view::Axis;
const WHOLE: Viewport = Viewport {
row_offset: 0,
col_offset: 0,
rows: 20,
cols: 20,
};
fn key(coords: &[(&str, &str)]) -> CellKey {
CellKey::new(
coords
.iter()
.map(|(c, i)| (c.to_string(), i.to_string()))
.collect(),
)
}
/// Region on Row, `_Measure` on Column, one record per cell.
fn regions(cells: &[(&str, &str, CellValue)]) -> Workbook {
let mut wb = Workbook::new("T");
wb.add_category("Region").unwrap();
for (region, measure, value) in cells {
wb.add_item("Region", *region).unwrap();
wb.add_item("_Measure", *measure).unwrap();
let at = key(&[("Region", region), ("_Measure", measure)]);
wb.model.set_cell(at, value.clone());
}
wb.active_view_mut().set_axis("Region", Axis::Row);
wb.active_view_mut().set_axis("_Measure", Axis::Column);
wb
}
fn total(wb: Workbook, measure: &str) -> CellDisplay {
let mut app = App::new(wb, None);
app.rebuild_layout();
let col = (0..app.layout.col_count())
.find(|&c| app.layout.col_label(c) == measure)
.unwrap_or_else(|| panic!("no column {measure}"));
grid_of(&app, WHOLE).totals[col].clone()
}
fn money(rows: &[(&str, f64, f64)]) -> Workbook {
let cells: Vec<_> = rows
.iter()
.flat_map(|(r, rev, cost)| {
[
(*r, "Revenue", CellValue::Number(*rev)),
(*r, "Cost", CellValue::Number(*cost)),
]
})
.collect();
regions(&cells)
}
/// improvise-wm20 — see context/test-evidence/command.md.
#[test]
fn a_ratio_columns_total_is_the_ratio_of_the_totalled_inputs() {
let mut wb = money(&[("North", 10.0, 5.0), ("South", 30.0, 3.0)]);
let margin =
crate::formula::parse_formula("Margin = (Revenue - Cost) / Revenue", "_Measure");
wb.model.add_formula(margin.unwrap());
// Rows are 0.50 and 0.90: summed 1.40, averaged 0.70, pooled 32/40.
assert_eq!(total(wb, "Margin").text, "0.80");
}
/// improvise-wm20: the fold is of the DRAWN rows, so a hidden row's values stay
/// out of every total, a formula's included. context/test-evidence/command.md.
#[test]
fn a_hidden_rows_values_stay_out_of_the_total() {
let mut wb = money(&[
("North", 10.0, 5.0),
("South", 30.0, 3.0),
("East", 100.0, 100.0),
]);
let margin =
crate::formula::parse_formula("Margin = (Revenue - Cost) / Revenue", "_Measure");
wb.model.add_formula(margin.unwrap());
wb.active_view_mut().hide_item("Region", "East");
assert_eq!(total(wb.clone(), "Revenue").text, "40.00");
assert_eq!(
total(wb, "Margin").text,
"0.80",
"East's 0 margin pooled in"
);
}
/// improvise-wm20: an Avg measure totals as the mean over its records, not
/// the sum or mean of the rows' means. context/test-evidence/command.md.
#[test]
fn an_average_measures_total_is_the_mean_over_its_records() {
let mut wb = Workbook::new("T");
wb.add_category("Region").unwrap();
wb.add_category("Product").unwrap();
wb.add_item("_Measure", "Price").unwrap();
for (region, product, price) in [
("North", "A", 10.0),
("North", "B", 20.0),
("South", "A", 30.0),
] {
wb.add_item("Region", region).unwrap();
wb.add_item("Product", product).unwrap();
let at = key(&[
("Region", region),
("Product", product),
("_Measure", "Price"),
]);
wb.model.set_cell(at, CellValue::Number(price));
}
let avg = crate::formula::AggFunc::Avg;
wb.model.measure_agg.insert("Price".into(), avg);
wb.active_view_mut().set_axis("Region", Axis::Row);
wb.active_view_mut().set_axis("_Measure", Axis::Column);
wb.active_view_mut().set_axis("Product", Axis::None);
// Rows are 15 and 30: summed 45, their mean 22.5, the records' mean 20.
assert_eq!(total(wb, "Price").text, "20.00");
}
/// improvise-wm20, ruling 1: a text column is unanimous or `mixed`.
#[test]
fn a_text_columns_total_is_its_text_when_every_row_agrees() {
let text = |s: &str| CellValue::Text(s.into());
let agreed = regions(&[
("North", "Status", text("ok")),
("South", "Status", text("ok")),
]);
let t = total(agreed, "Status");
assert_eq!((t.text.as_str(), t.error), ("ok", false));
let split = regions(&[
("North", "Status", text("ok")),
("South", "Status", text("no")),
]);
let t = total(split, "Status");
assert!(t.error, "differing text totals as an error: {t:?}");
}
/// improvise-wm20, ruling 2: an error in any row makes the total an error.
#[test]
fn an_error_in_any_row_makes_the_total_an_error() {
let wb = regions(&[
("North", "Revenue", CellValue::Number(10.0)),
("South", "Revenue", CellValue::Error("bad".into())),
]);
let t = total(wb, "Revenue");
assert!(t.error, "a row's error was dropped from the total: {t:?}");
}
/// improvise-wm20, ruling 3: with `_Measure` on the row axis a column adds
/// different measures together, so there is no strip at all.
#[test]
fn measures_on_the_row_axis_leave_no_totals_strip() {
let mut wb = money(&[("North", 10.0, 5.0), ("South", 30.0, 3.0)]);
wb.active_view_mut().set_axis("Region", Axis::Column);
wb.active_view_mut().set_axis("_Measure", Axis::Row);
let mut app = App::new(wb, None);
app.rebuild_layout();
assert!(grid_of(&app, WHOLE).totals.is_empty());
}
/// improvise-wm20 with #203's folds: a Max measure's total does not move when
/// a row group collapses. context/test-evidence/command.md.
#[test]
fn a_max_measures_total_does_not_move_when_a_row_group_collapses() {
let book = |collapsed: bool| {
let mut wb = Workbook::new("T");
wb.add_category("Month").unwrap();
wb.add_category("Region").unwrap();
wb.add_item("_Measure", "Revenue").unwrap();
for (month, n, s) in [("Jan", 5.0, 1.0), ("Feb", 2.0, 7.0)] {
wb.add_item_in_group("Month", month, "Q1").unwrap();
for (region, v) in [("North", n), ("South", s)] {
wb.add_item("Region", region).unwrap();
let at = key(&[
("Month", month),
("Region", region),
("_Measure", "Revenue"),
]);
wb.model.set_cell(at, CellValue::Number(v));
}
}
let max = crate::formula::AggFunc::Max;
wb.model.measure_agg.insert("Revenue".into(), max);
wb.active_view_mut().set_axis("Month", Axis::Row);
wb.active_view_mut().set_axis("_Measure", Axis::Column);
wb.active_view_mut().set_axis("Region", Axis::None);
if collapsed {
wb.active_view_mut().toggle_group_collapse("Month", "Q1");
}
wb
};
// Expanded rows are 5 and 7, which a strip that sums reads as 12.
assert_eq!(total(book(false), "Revenue").text, "7.00");
assert_eq!(total(book(true), "Revenue").text, "7.00");
}
/// The column's grid and its index, for a test that reads rows and total together.
fn column(wb: Workbook, measure: &str) -> (GridCache, usize) {
let mut app = App::new(wb, None);
app.rebuild_layout();
let col = (0..app.layout.col_count())
.find(|&c| app.layout.col_label(c) == measure)
.unwrap_or_else(|| panic!("no column {measure}"));
(grid_of(&app, WHOLE), col)
}
/// North 25/30, South 27/30, East -4/0: the rows draw 0.83, 0.90, div/0.
fn margins_with_a_zero_revenue_row() -> Workbook {
money(&[
("North", 30.0, 5.0),
("South", 30.0, 3.0),
("East", 0.0, 4.0),
])
}
/// improvise-b5dp, user ruling: a formula total is the pooled ratio, so a row's
/// div/0 does not poison it. context/test-evidence/command.md.
#[test]
fn a_formula_rows_error_does_not_poison_the_pooled_total() {
let mut wb = margins_with_a_zero_revenue_row();
for f in ["Profit = Revenue - Cost", "Margin = Profit / Revenue"] {
let formula = crate::formula::parse_formula(f, "_Measure").unwrap();
wb.model.add_formula(formula);
}
let (grid, col) = column(wb, "Margin");
let rows: Vec<bool> = grid.cells.iter().map(|r| r[col].error).collect();
assert_eq!(rows, vec![false, false, true], "East's row must draw div/0");
let t = &grid.totals[col];
assert_eq!(
(t.text.as_str(), t.error),
("0.80", false),
"48 / 60, pooled"
);
}
/// improvise-b5dp: the same ruling for a formula whose own aggregate is zero
/// at one row. context/test-evidence/command.md.
#[test]
fn a_share_columns_total_pools_past_a_rows_div_zero() {
let mut wb = margins_with_a_zero_revenue_row();
let share = crate::formula::parse_formula("Share = Revenue / SUM(Revenue)", "_Measure");
wb.model.add_formula(share.unwrap());
let (grid, col) = column(wb, "Share");
assert!(grid.cells[2][col].error, "East's row must draw div/0");
let t = &grid.totals[col];
assert_eq!(
(t.text.as_str(), t.error),
("1.00", false),
"60 / 60, pooled"
);
}
}
crates/improvise-command/src/ui/reduce.rs
+15
View File
@@ -654,6 +654,8 @@ pub(crate) mod tests {
/// resolves — otherwise every walk below fails for a reason not its own.
fn model_app() -> App {
let mut app = app();
let workbook = &mut app.model_state.workbook;
workbook.add_item(MEASURE_CATEGORY, "Revenue").unwrap();
app.model_state
.workbook
.model
@@ -959,6 +961,16 @@ pub(crate) mod tests {
]);
wb.model.set_cell(key, CellValue::Number(n as f64 + 1.0));
}
// Several entries, so two instances' HashMaps can disagree on order (improvise-m6op).
use crate::formula::AggFunc;
for (measure, agg) in [
("Revenue", AggFunc::Avg),
("Cost", AggFunc::Min),
("Units", AggFunc::Max),
("Orders", AggFunc::Count),
] {
wb.model.measure_agg.insert(measure.into(), agg);
}
let view = wb.active_view_mut();
view.set_axis("Region", Axis::Row);
view.set_axis("Month", Axis::Column);
@@ -1101,6 +1113,9 @@ pub(crate) mod tests {
},
] {
let mut app = app();
let workbook = &mut app.model_state.workbook;
workbook.add_item(MEASURE_CATEGORY, "Revenue").unwrap();
app.rebuild_layout();
assert!(!app.model_state.dirty, "the fixture starts clean");
let wire = caught_up(&app, cmd.clone());
apply_checked(&mut app, &wire);
crates/improvise-command/src/ui/viewport.rs
+21 -12
View File
@@ -7,7 +7,9 @@ use improvise_protocol::header_level_widths;
use crate::format::{FormatPlan, NumberFormat};
use crate::model::Model;
use crate::model::category::MEASURE_CATEGORY;
use crate::model::cell::{CellKey, CellValue};
use crate::model::types::FoldMemo;
use crate::view::{AxisEntry, GridLayout};
/// Minimum column width — enough for short numbers/labels + 1 char gap.
@@ -26,9 +28,9 @@ pub struct GridBody {
/// The format each column's TOTAL is drawn with, resolved from the
/// coordinates that survive the row aggregation — `context/code/core.md`.
pub col_formats: Vec<NumberFormat>,
/// Column sums over every row. **Empty** in records mode or an empty grid,
/// which is "this grid has no totals" rather than a strip of zeroes.
pub totals: Vec<f64>,
/// Each column's total over every drawn row (improvise-wm20). **Empty** in
/// records mode, an empty grid, or with `_Measure` on the row axis.
pub totals: Vec<Option<CellValue>>,
/// The requested window, row-major. `None` is a cell the layout has not got.
pub window: Vec<Vec<Option<CellValue>>>,
}
@@ -45,9 +47,16 @@ pub fn grid_body(
let n = layout.col_count();
let records = layout.is_records_mode();
let mut widths = header_level_seed(layout, n);
let totalled = !records && layout.row_count() > 0 && n > 0;
let mut totals = match totalled {
true => vec![0.0f64; n],
// One memo for the whole build: a fold every cell and total reaches is
// evaluated once (improvise-h6z1). The model does not change during it.
let memo = FoldMemo::default();
// A column mixing measures has no total to show: `context/code/command.md`.
let measures_on_rows = layout.row_cats.iter().any(|c| c == MEASURE_CATEGORY);
let totalled = !records && !measures_on_rows && layout.row_count() > 0 && n > 0;
let totals: Vec<Option<CellValue>> = match totalled {
true => (0..n)
.map(|ci| layout.column_total(model, ci, &memo))
.collect(),
false => Vec::new(),
};
let mut window: Vec<Vec<Option<CellValue>>> = Vec::with_capacity(rows.len());
@@ -64,16 +73,13 @@ pub fn grid_body(
false => Vec::new(),
};
for ci in 0..n {
let (key, value) = cell_value(model, layout, records, ri, ci);
let (key, value) = cell_value(model, layout, records, ri, ci, &memo);
let fmt = plan.at(key.as_ref());
let text = match records {
true => value.as_ref().map(|v| v.to_string()).unwrap_or_default(),
false => fmt.value(value.as_ref()),
};
widen(&mut widths, ci, text.width());
if let Some(sum) = totals.get_mut(ci) {
*sum += value.as_ref().and_then(|v| v.as_f64()).unwrap_or(0.0);
}
if drawn && cols.contains(&ci) {
row_out.push(value);
}
@@ -84,7 +90,7 @@ pub fn grid_body(
}
for (ci, total) in totals.iter().enumerate() {
let fmt = col_formats.get(ci).copied().unwrap_or(plan.view_format());
widen(&mut widths, ci, fmt.number(*total).width());
widen(&mut widths, ci, fmt.value(total.as_ref()).width());
}
GridBody {
@@ -105,12 +111,15 @@ fn cell_value(
records: bool,
row: usize,
col: usize,
memo: &FoldMemo,
) -> (Option<CellKey>, Option<CellValue>) {
match records {
true => (None, layout.records_value(row, col)),
false => {
let key = layout.cell_key(row, col);
let value = key.as_ref().and_then(|k| layout.evaluate(model, k));
let value = key
.as_ref()
.and_then(|k| layout.evaluate_in(model, k, memo));
(key, value)
}
}
crates/improvise-core/src/model/cell.rs
+175 -7
View File
@@ -378,16 +378,30 @@ impl DataStore {
/// Positions in `cells` of every cell matching `partial`, **in insertion
/// order**. The index narrows; the primary map orders (§8).
fn matching_positions(&self, partial: &[(String, String)]) -> Vec<usize> {
// Intern the partial key (lookup only, no new symbols)
let interned_partial: Vec<(Symbol, Symbol)> = partial
self.intern_partial(partial)
.map(|pairs| self.positions_of(&pairs))
.unwrap_or_default()
}
/// `partial` as symbols, or `None` when a name was never interned, which
/// no stored cell can then match.
fn intern_partial(&self, partial: &[(String, String)]) -> Option<Vec<(Symbol, Symbol)>> {
partial
.iter()
.filter_map(|(c, i)| Some((self.symbols.get(c)?, self.symbols.get(i)?)))
.collect();
.map(|(c, i)| Some((self.symbols.get(c)?, self.symbols.get(i)?)))
.collect()
}
if interned_partial.len() < partial.len() {
return vec![];
}
/// How many candidates narrowing by `pairs` would visit: its smallest index set.
fn narrowing_cost(&self, pairs: &[(Symbol, Symbol)]) -> usize {
pairs
.iter()
.map(|pair| self.index.get(pair).map_or(0, HashSet::len))
.min()
.unwrap_or(self.cells.len())
}
fn positions_of(&self, interned_partial: &[(Symbol, Symbol)]) -> Vec<usize> {
let mut sets: Vec<&HashSet<InternedKey>> = interned_partial
.iter()
.filter_map(|pair| self.index.get(pair))
@@ -426,6 +440,71 @@ impl DataStore {
.collect()
}
/// The union of `matching_values` (or, `exact`, of `get`) over every key `partial`
/// extends by one listed item per spanned category — scanned once (improvise-yb4b).
pub fn spanned_values(
&self,
partial: &[(String, String)],
spans: &[(&str, Vec<&str>)],
exact: bool,
) -> Vec<&CellValue> {
let wanted: Option<Vec<(Symbol, HashSet<Symbol>)>> = spans
.iter()
.map(|(cat, items)| {
let items = items.iter().filter_map(|i| self.symbols.get(i)).collect();
Some((self.symbols.get(cat)?, items))
})
.collect();
let Some(wanted) = wanted else {
return Vec::new();
};
let admits = |ikey: &InternedKey| {
(!exact || ikey.0.len() == partial.len() + wanted.len())
&& wanted
.iter()
.all(|(cat, items)| ikey.0.iter().any(|(c, i)| c == cat && items.contains(i)))
};
self.spanned_positions(partial, &wanted)
.into_iter()
.filter_map(|n| self.cells.get_index(n))
.filter(|(ikey, _)| admits(ikey))
.map(|(_, value)| value)
.collect()
}
/// Candidates for a span: narrowed once by `partial`, or once per covered item
/// of its smallest span, whichever visits fewer. Sorted (improvise-jq0m).
fn spanned_positions(
&self,
partial: &[(String, String)],
wanted: &[(Symbol, HashSet<Symbol>)],
) -> Vec<usize> {
self.intern_partial(partial)
.map(|base| self.narrowed(&base, wanted))
.unwrap_or_default()
}
fn narrowed(
&self,
base: &[(Symbol, Symbol)],
wanted: &[(Symbol, HashSet<Symbol>)],
) -> Vec<usize> {
let with = |cat: Symbol, item: Symbol| [base, &[(cat, item)]].concat();
let per_item: Vec<Vec<(Symbol, Symbol)>> = wanted
.iter()
.min_by_key(|(_, items)| items.len())
.map(|(cat, items)| items.iter().map(|i| with(*cat, *i)).collect())
.unwrap_or_default();
let item_cost: usize = per_item.iter().map(|p| self.narrowing_cost(p)).sum();
let narrow_per_item = !per_item.is_empty() && item_cost < self.narrowing_cost(base);
let mut positions: Vec<usize> = match narrow_per_item {
true => per_item.iter().flat_map(|p| self.positions_of(p)).collect(),
false => self.positions_of(base),
};
positions.sort_unstable();
positions
}
/// Every cell matching `partial`, in insertion order. Allocates a
/// `CellKey` per match — prefer `matching_values` when values suffice.
#[allow(dead_code)]
@@ -769,6 +848,41 @@ mod data_store {
"the fast path and the general path must agree"
);
}
/// improvise-yb4b: one scan answers what a lookup per member answered, in both
/// modes. context/test-evidence/core.md.
#[test]
fn a_spanned_scan_finds_what_a_lookup_per_member_finds() {
let mut store = DataStore::new();
let cells: [(&[(&str, &str)], f64); 7] = [
(&[("M", "Rev"), ("R", "North"), ("P", "A")], 1.0),
(&[("M", "Rev"), ("R", "South"), ("P", "A")], 2.0),
(&[("M", "Rev"), ("R", "East"), ("P", "A")], 4.0),
(&[("M", "Rev"), ("R", "North"), ("P", "A"), ("X", "x")], 8.0),
(&[("M", "Rev"), ("P", "A")], 16.0),
(&[("M", "Cost"), ("R", "North"), ("P", "A")], 32.0),
(&[("M", "Rev"), ("R", "North"), ("P", "B")], 64.0),
];
for (coords, v) in cells {
store.set(key(coords), CellValue::Number(v));
}
let base = key(&[("M", "Rev"), ("P", "A")]);
let covered = vec!["North", "South", "West"];
let members: Vec<CellKey> = covered.iter().map(|r| base.clone().with("R", *r)).collect();
let sum = |values: Vec<&CellValue>| values.iter().filter_map(|v| v.as_f64()).sum::<f64>();
let per_member_exact = members.iter().filter_map(|k| store.get(k)).collect();
let per_member_over = members
.iter()
.flat_map(|k| store.matching_values(&k.0))
.collect();
let spans = [("R", covered.clone())];
let scan_exact = store.spanned_values(&base.0, &spans, true);
let scan_over = store.spanned_values(&base.0, &spans, false);
assert_eq!((sum(scan_exact), sum(per_member_exact)), (3.0, 3.0));
assert_eq!((sum(scan_over), sum(per_member_over)), (11.0, 11.0));
let unknown = [("Nowhere", covered)];
assert!(store.spanned_values(&base.0, &unknown, false).is_empty());
}
}
#[cfg(test)]
@@ -941,6 +1055,60 @@ mod prop_tests {
"returned key {result_key} does not match partial {partial:?}");
}
}
/// improvise-jq0m: whichever narrowing it picks, a span returns what a lookup
/// per member returns, in store order. context/test-evidence/core.md.
#[test]
fn a_span_finds_what_its_members_find_in_store_order(
cells in prop::collection::vec((0u8..6, 0u8..3, 0u8..2, any::<bool>()), 0..40),
covered_r in prop::collection::vec(any::<bool>(), 7),
covered_p in prop::option::of(prop::collection::vec(any::<bool>(), 3)),
base_m in 0u8..2,
exact in any::<bool>(),
) {
let mut store = DataStore::new();
for (n, (r, p, m, extra)) in cells.iter().enumerate() {
let mut key = CellKey::new(vec![
("P".into(), format!("p{p}")),
("M".into(), format!("m{m}")),
]);
// r = 5 leaves the record without the spanned category at all.
key = match r { 5 => key, r => key.with("R", format!("r{r}")) };
key = match extra { true => key.with("X", "x"), false => key };
store.set(key, CellValue::Number(n as f64));
}
// r6 is never stored, so it is a covered item the store never interned.
let pick = |prefix: &str, flags: &[bool]| -> Vec<String> {
flags.iter().enumerate().filter(|(_, f)| **f)
.map(|(i, _)| format!("{prefix}{i}")).collect()
};
let rs = pick("r", &covered_r);
let ps = covered_p.as_deref().map(|f| pick("p", f));
let base = CellKey::new(vec![("M".into(), format!("m{base_m}"))]);
let mut spans: Vec<(&str, Vec<&str>)> = vec![("R", rs.iter().map(String::as_str).collect())];
if let Some(ps) = &ps {
spans.push(("P", ps.iter().map(String::as_str).collect()));
}
let mut members = vec![base.clone()];
for (cat, items) in &spans {
members = members.iter()
.flat_map(|k| items.iter().map(|i| k.clone().with(*cat, *i)))
.collect();
}
let matched: std::collections::HashSet<CellKey> = members.iter()
.flat_map(|k| match exact {
true => store.get(k).map(|_| k.clone()).into_iter().collect::<Vec<_>>(),
false => store.matching_cells(&k.0).into_iter().map(|(k, _)| k).collect(),
})
.collect();
let expected: Vec<CellValue> = store.iter_cells()
.filter(|(k, _)| matched.contains(k))
.map(|(_, v)| v.clone())
.collect();
let got: Vec<CellValue> = store.spanned_values(&base.0, &spans, exact)
.into_iter().cloned().collect();
prop_assert_eq!(got, expected);
}
}
}
crates/improvise-core/src/model/types.rs
+148 -59
View File
@@ -140,17 +140,62 @@ fn agg_values(
// How many formulas a fold has evaluated: what improvise-o1y0's regression counts.
#[cfg(test)]
thread_local! {
static FOLD_FORMULA_EVALUATIONS: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
pub(crate) static FOLD_FORMULA_EVALUATIONS: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
}
/// One fold evaluation's memory: each formula key is evaluated once, and a key met
/// while still in progress (`None`) is a cycle (improvise-o1y0).
type FoldMemo = std::cell::RefCell<HashMap<CellKey, Option<Option<CellValue>>>>;
/// Each (fold key, span) evaluated once across one grid build (improvise-o1y0,
/// improvise-h6z1). Valid for one model state only: `context/code/core.md` §14.
#[derive(Default)]
pub struct FoldMemo(std::cell::RefCell<HashMap<(CellKey, Vec<String>), MemoEntry>>);
/// `None` while in progress, which is how a cycle is seen (improvise-o1y0).
type MemoEntry = Option<Option<CellValue>>;
impl FoldMemo {
fn get(&self, key: &CellKey, span: Span) -> Option<MemoEntry> {
let at = (key.clone(), span.over.to_vec());
self.0.borrow().get(&at).cloned()
}
fn put(&self, key: &CellKey, span: Span, entry: MemoEntry) {
let at = (key.clone(), span.over.to_vec());
self.0.borrow_mut().insert(at, entry);
}
}
/// Which members of a collapsed group its fold covers. The model knows no
/// view, so the grid passes the members it draws (core.md §14).
pub type Covers<'a> = &'a dyn Fn(&str, &str) -> bool;
/// What a fold reaches: the covered members of each group its key names, and
/// every covered item of each category in `over` its key omits (improvise-wm20).
#[derive(Clone, Copy)]
struct Span<'a> {
covers: Covers<'a>,
over: &'a [String],
}
/// A fold's reach as data: `base` is the key with each expanded category removed,
/// and `spans` the covered items of each, in expansion order (improvise-yb4b).
struct Expansion<'a> {
base: CellKey,
spans: Vec<(&'a str, Vec<&'a str>)>,
}
impl Expansion<'_> {
/// One key per member, the cross product in expansion order.
fn members(&self) -> Vec<CellKey> {
let mut expanded = vec![self.base.clone()];
for (cat_name, covered) in &self.spans {
expanded = expanded
.iter()
.flat_map(|k| covered.iter().map(|m| k.clone().with(*cat_name, *m)))
.collect();
}
expanded
}
}
/// Where a formula's references are read: at one record, or across the members
/// a collapsed group's fold covers, as a None axis reads them (core.md §14).
#[derive(Clone, Copy)]
@@ -158,7 +203,7 @@ enum Reach<'a> {
Record,
Fold {
agg: Aggregation,
covers: Covers<'a>,
span: Span<'a>,
memo: &'a FoldMemo,
},
}
@@ -184,9 +229,7 @@ fn eval_expr(
let new_key = context.clone().with(cat, name);
let value = match reach {
Reach::Record => model.evaluate_depth(&new_key, depth),
Reach::Fold { agg, covers, memo } => {
model.evaluate_fold(&new_key, agg, covers, memo)
}
Reach::Fold { agg, span, memo } => model.evaluate_fold(&new_key, agg, span, memo),
};
match value {
Some(CellValue::Number(n)) => Ok(n),
@@ -654,7 +697,46 @@ impl Model {
agg: Aggregation,
covers: Covers,
) -> Option<CellValue> {
self.evaluate_fold(key, agg, covers, &FoldMemo::default())
let span = Span { covers, over: &[] };
self.evaluate_fold(key, agg, span, &FoldMemo::default())
}
/// [`Self::evaluate_covering`] sharing `memo` with every other evaluation of one
/// grid build, which must not outlive the model state it read (improvise-h6z1).
pub fn evaluate_covering_in(
&self,
key: &CellKey,
agg: Aggregation,
covers: Covers,
memo: &FoldMemo,
) -> Option<CellValue> {
let span = Span { covers, over: &[] };
self.evaluate_fold(key, agg, span, memo)
}
/// The value `key` has as ONE fold over every covered item of the `over`
/// categories it omits — a column's total (improvise-wm20, core.md §14).
pub fn evaluate_spanning(
&self,
key: &CellKey,
over: &[String],
agg: Aggregation,
covers: Covers,
) -> Option<CellValue> {
self.evaluate_spanning_in(key, over, agg, covers, &FoldMemo::default())
}
/// [`Self::evaluate_spanning`] sharing one grid build's `memo` (improvise-h6z1).
pub fn evaluate_spanning_in(
&self,
key: &CellKey,
over: &[String],
agg: Aggregation,
covers: Covers,
memo: &FoldMemo,
) -> Option<CellValue> {
let span = Span { covers, over };
self.evaluate_fold(key, agg, span, memo)
}
/// A key naming a group folds; any other key reads as before. A formula is
@@ -663,16 +745,16 @@ impl Model {
&self,
key: &CellKey,
agg: Aggregation,
covers: Covers,
span: Span,
memo: &FoldMemo,
) -> Option<CellValue> {
let members = self.group_expansion(key, covers);
match (members.is_empty(), self.formula_for(key)) {
let expansion = self.group_expansion(key, span);
match (expansion.spans.is_empty(), self.formula_for(key)) {
(true, _) => self.evaluate_unfolded(key, agg),
(false, Some(formula)) => {
self.fold_formula_once(formula, key, &members, (agg, covers, memo))
self.fold_formula_once(formula, key, &expansion, (agg, span, memo))
}
(false, None) => self.fold_stored(key, &members, agg),
(false, None) => self.fold_stored(key, &expansion, agg),
}
}
@@ -682,20 +764,20 @@ impl Model {
&self,
formula: &Formula,
key: &CellKey,
members: &[CellKey],
(agg, covers, memo): (Aggregation, Covers, &FoldMemo),
expansion: &Expansion,
(agg, span, memo): (Aggregation, Span, &FoldMemo),
) -> Option<CellValue> {
let seen = memo.borrow().get(key).cloned();
let seen = memo.get(key, span);
match seen {
Some(Some(value)) => value,
Some(None) => Some(CellValue::Error("circular".into())),
None => {
memo.borrow_mut().insert(key.clone(), None);
memo.put(key, span, None);
#[cfg(test)]
FOLD_FORMULA_EVALUATIONS.with(|n| n.set(n.get() + 1));
let reach = Reach::Fold { agg, covers, memo };
let value = self.eval_formula_folded(formula, key, members, agg, reach);
memo.borrow_mut().insert(key.clone(), Some(value.clone()));
let reach = Reach::Fold { agg, span, memo };
let value = self.eval_formula_folded(formula, key, expansion, agg, reach);
memo.put(key, span, Some(value.clone()));
value
}
}
@@ -712,31 +794,40 @@ impl Model {
aggregate_cell_values(&self.data.matching_values(&key.0), self.agg_for(key))
}
/// One key per covered member where `key` names a collapsed GROUP rather than
/// an item — the cross product over every such coordinate. Empty when none does.
fn group_expansion(&self, key: &CellKey, covers: Covers) -> Vec<CellKey> {
let mut expanded = vec![key.clone()];
for (cat_name, name) in &key.0 {
/// Each category `key` folds, with its covered members: a collapsed GROUP it
/// names, or a spanned category it omits. No spans when neither.
fn group_expansion<'a>(&'a self, key: &'a CellKey, span: Span<'a>) -> Expansion<'a> {
let grouped = key.0.iter().map(|(cat_name, name)| {
let members: Vec<&str> = self
.category(cat_name)
.filter(|cat| !cat.items.contains_key(name.as_str()))
.map(|cat| cat.items_in_group(name))
.unwrap_or_default()
.into_iter()
.filter(|m| covers(cat_name, m))
.collect();
if members.is_empty() {
continue;
}
expanded = expanded
.iter()
.flat_map(|k| members.iter().map(|m| k.clone().with(cat_name, *m)))
.collect();
}
match expanded.as_slice() {
[only] if only == key => Vec::new(),
_ => expanded,
}
.unwrap_or_default();
(cat_name.as_str(), members)
});
let spanned = span
.over
.iter()
.filter(|cat_name| key.get(cat_name).is_none())
.map(|cat_name| {
let items = self.category(cat_name).map(|cat| cat.ordered_item_names());
(cat_name.as_str(), items.unwrap_or_default())
});
let spans: Vec<(&str, Vec<&str>)> = grouped
.chain(spanned)
.map(|(cat_name, members)| {
let covered: Vec<&str> = members
.into_iter()
.filter(|m| (span.covers)(cat_name, m))
.collect();
(cat_name, covered)
})
.filter(|(_, covered)| !covered.is_empty())
.collect();
let base = spans
.iter()
.fold(key.clone(), |k, (cat_name, _)| k.without(cat_name));
Expansion { base, spans }
}
/// A coordinate as a formula reads it. At a fold a group's name is no item, so
@@ -762,30 +853,28 @@ impl Model {
/// The contexts an aggregate inside a formula pools over at `context`.
fn contexts_of(&self, context: &CellKey, reach: Reach) -> Vec<CellKey> {
let members = match reach {
Reach::Record => Vec::new(),
Reach::Fold { covers, .. } => self.group_expansion(context, covers),
let expansion = match reach {
Reach::Record => None,
Reach::Fold { span, .. } => Some(self.group_expansion(context, span)),
};
match members.is_empty() {
true => vec![context.clone()],
false => members,
match expansion.filter(|e| !e.spans.is_empty()) {
None => vec![context.clone()],
Some(e) => e.members(),
}
}
/// A group's stored values fold ONCE over every record the group covers.
/// A group's stored values fold ONCE over every record the group covers, in
/// the store's own order: one scan, not a lookup per member (improvise-yb4b).
fn fold_stored(
&self,
key: &CellKey,
members: &[CellKey],
expansion: &Expansion,
agg: Aggregation,
) -> Option<CellValue> {
let values: Vec<&CellValue> = members
.iter()
.flat_map(|k| match agg {
Aggregation::Exact => self.data.get(k).into_iter().collect(),
Aggregation::OverHidden => self.data.matching_values(&k.0),
})
.collect();
let exact = agg == Aggregation::Exact;
let values = self
.data
.spanned_values(&expansion.base.0, &expansion.spans, exact);
aggregate_cell_values(&values, self.agg_for(key))
}
@@ -795,7 +884,7 @@ impl Model {
&self,
formula: &Formula,
key: &CellKey,
members: &[CellKey],
expansion: &Expansion,
agg: Aggregation,
reach: Reach,
) -> Option<CellValue> {
@@ -804,7 +893,7 @@ impl Model {
.as_ref()
.is_none_or(|f| self.read_coordinate(key, &f.category, reach) == Some(f.item.as_str()));
if !applies {
return self.fold_stored(key, members, agg);
return self.fold_stored(key, expansion, agg);
}
let depth = Self::MAX_EVAL_DEPTH;
match eval_expr(
crates/improvise-core/src/view/layout.rs
+57 -1
View File
@@ -3,6 +3,7 @@ use std::rc::Rc;
use crate::model::category::{DIM_CATEGORY, INDEX_CATEGORY, MEASURE_CATEGORY};
use crate::model::cell::{CellKey, CellValue};
use crate::model::types::FoldMemo;
use crate::model::{Aggregation, Model};
use crate::view::{Axis, View};
@@ -657,8 +658,22 @@ impl GridLayout {
/// The value of a pivot key as THIS grid reads it: aggregated over its None
/// axis, and a fold over the members it draws (core.md §14). One evaluation.
pub fn evaluate(&self, model: &Model, key: &CellKey) -> Option<CellValue> {
self.evaluate_in(model, key, &FoldMemo::default())
}
/// [`Self::evaluate`] sharing one grid build's `memo` (improvise-h6z1).
pub fn evaluate_in(&self, model: &Model, key: &CellKey, memo: &FoldMemo) -> Option<CellValue> {
let drawn = |cat: &str, item: &str| !self.hidden.get(cat).is_some_and(|h| h.contains(item));
model.evaluate_covering(key, Aggregation::of(&self.none_cats), &drawn)
model.evaluate_covering_in(key, Aggregation::of(&self.none_cats), &drawn, memo)
}
/// A column's total: its cell as if every drawn row were ONE fold, so a
/// formula is evaluated over the pooled rows (improvise-wm20, core.md §14).
pub fn column_total(&self, model: &Model, col: usize, memo: &FoldMemo) -> Option<CellValue> {
let key = self.column_key(col)?;
let drawn = |cat: &str, item: &str| !self.hidden.get(cat).is_some_and(|h| h.contains(item));
let agg = Aggregation::of(&self.none_cats);
model.evaluate_spanning_in(&key, &self.row_cats, agg, &drawn, memo)
}
/// Display text for (row, col) in either mode.
@@ -895,6 +910,7 @@ mod tests {
prune_axis_items, synthetic_record_info,
};
use crate::model::cell::{CellKey, CellValue};
use crate::model::types::{FOLD_FORMULA_EVALUATIONS, FoldMemo};
use crate::view::Axis;
use crate::workbook::Workbook;
@@ -1832,6 +1848,46 @@ mod tests {
.and_then(|v| v.as_f64())
}
/// improvise-h6z1: one grid build evaluates each formula a fold row reaches
/// once, not once per column. context/test-evidence/core.md.
#[test]
fn a_fold_row_evaluates_each_formula_once_across_the_row() {
let chain: Vec<String> = std::iter::once("F0 = Revenue".to_string())
.chain((1..=8).map(|i| format!("F{i} = F{p} + F{p}", p = i - 1)))
.collect();
let chain: Vec<&str> = chain.iter().map(String::as_str).collect();
let mut wb = margin_book(&chain);
wb.active_view_mut().toggle_group_collapse("Month", "Q1");
wb.model.recompute_formulas(&wb.active_view().none_cats());
let layout = GridLayout::new(&wb.model, wb.active_view());
let q1 = (0..layout.row_count())
.find(|&r| layout.row_label(r) == "Q1")
.expect("a Q1 fold row");
let row_keys: Vec<CellKey> = (0..layout.col_count())
.filter_map(|c| layout.cell_key(q1, c))
.collect();
let evaluations = |shared: bool| {
let memo = FoldMemo::default();
FOLD_FORMULA_EVALUATIONS.with(|n| n.set(0));
let values: Vec<_> = row_keys
.iter()
.map(|k| match shared {
true => layout.evaluate_in(&wb.model, k, &memo),
false => layout.evaluate(&wb.model, k),
})
.collect();
(FOLD_FORMULA_EVALUATIONS.with(|n| n.get()), values)
};
let (apart, separately) = evaluations(false);
let (together, shared) = evaluations(true);
assert_eq!(shared, separately, "sharing the memo changed a value");
assert_eq!(
apart, 45,
"setup: each column re-derives the chain below it"
);
assert_eq!(together, 9, "F0..F8 once each across the whole row");
}
/// improvise-eccl — see context/test-evidence/core.md.
#[test]
fn a_folds_ratio_formula_is_the_ratio_of_its_folded_inputs() {
crates/improvise-formula/src/parser.rs
+37 -8
View File
@@ -1167,9 +1167,23 @@ mod generator {
use pest_meta::parser;
use proptest::prelude::*;
use std::collections::HashMap;
use std::sync::OnceLock;
use std::sync::atomic::{AtomicUsize, Ordering};
/// Counts how often formula.pest is actually parsed; the guard below keeps it
/// from becoming a per-case parse again.
static GRAMMAR_PARSES: AtomicUsize = AtomicUsize::new(0);
/// The parsed grammar, read once for the process. Evidence:
/// `context/test-evidence/formula-and-io.md`.
fn grammar() -> &'static HashMap<String, (RuleType, Expr)> {
static RULES: OnceLock<HashMap<String, (RuleType, Expr)>> = OnceLock::new();
RULES.get_or_init(load_grammar)
}
/// Parse formula.pest and return rules keyed by name.
fn load_grammar() -> HashMap<String, (RuleType, Expr)> {
GRAMMAR_PARSES.fetch_add(1, Ordering::Relaxed);
let grammar = include_str!("formula.pest");
let pairs = parser::parse(parser::Rule::grammar_rules, grammar)
.unwrap_or_else(|e| panic!("Bad grammar: {e}"));
@@ -1301,18 +1315,33 @@ mod generator {
/// Proptest strategy: generate a random valid formula by walking the
/// grammar's `formula` rule.
pub fn formula_string() -> impl Strategy<Value = String> {
prop::collection::vec(any::<u8>(), 32..=128).prop_map(|choices| {
let rules = load_grammar();
Gen::new(&rules, choices).generate("formula")
})
prop::collection::vec(any::<u8>(), 32..=128)
.prop_map(|choices| Gen::new(grammar(), choices).generate("formula"))
}
/// Proptest strategy: generate a random valid standalone expression.
pub fn expr_string() -> impl Strategy<Value = String> {
prop::collection::vec(any::<u8>(), 32..=128).prop_map(|choices| {
let rules = load_grammar();
Gen::new(&rules, choices).generate("expr_eoi")
})
prop::collection::vec(any::<u8>(), 32..=128)
.prop_map(|choices| Gen::new(grammar(), choices).generate("expr_eoi"))
}
/// improvise-i5z6. Evidence: `context/test-evidence/formula-and-io.md`.
#[test]
fn the_grammar_is_parsed_once_however_many_formulas_the_strategies_yield() {
use proptest::strategy::ValueTree;
use proptest::test_runner::TestRunner;
let mut runner = TestRunner::deterministic();
let (formulas, exprs) = (formula_string(), expr_string());
for _ in 0..32 {
let _ = formulas.new_tree(&mut runner).unwrap().current();
let _ = exprs.new_tree(&mut runner).unwrap().current();
}
assert_eq!(
GRAMMAR_PARSES.load(Ordering::Relaxed),
1,
"formula.pest was re-parsed per generated case"
);
}
}
crates/improvise-protocol/src/client.rs
+115
View File
@@ -113,6 +113,8 @@ impl ClientState {
}
}
self.settle_sent();
self.settle_editor();
self.revision += 1;
}
/// The host applied **none** of the oldest awaited frame (ADR 001 clause
@@ -175,6 +177,23 @@ impl ClientState {
self.view.buffers.insert(key, seed.clone());
self.view.opened_editor = Some(crate::OpenedEditor::on(seed));
}
self.settle_awaited_seed();
}
/// An editor opened after a write takes its seed from the grid the host
/// answered with, never from the cache the write made stale (improvise-3uzg).
fn settle_awaited_seed(&mut self) {
let waiting = self.view.mode.is_editing()
&& self
.view
.opened_editor
.as_ref()
.is_some_and(|opened| opened.awaiting && !opened.touched);
let answered = !self.in_flight.iter().any(|sent| sent.awaited);
if let (true, true, Some(seed)) = (waiting, answered, crate::resolve::edit_seed(self)) {
self.view.buffers.insert("edit".to_string(), seed.clone());
self.view.opened_editor = Some(crate::OpenedEditor::on(seed));
}
}
/// The widget dispatch: a projection names the widget it is addressed to.
@@ -398,6 +417,102 @@ mod tests {
);
}
/// A write, then the editor its advance reopens with the seed still to come.
fn write_then_awaiting_editor() -> Vec<Command> {
let mut frame = commit_like("written");
frame.extend([
Command::SetBuffer {
name: "edit".into(),
value: String::new(),
},
Command::SetOpenedEditor(Some(crate::OpenedEditor::awaiting())),
Command::ChangeMode(crate::AppMode::editing()),
]);
frame
}
/// One row, one column, the cell's stored text `edit_text`.
fn answered_with(state: &mut ClientState, edit_text: &str) {
let mut cell = CellDisplay::number(edit_text);
cell.edit_text = Some(edit_text.into());
for cmd in [
GridUpdate::RowFrame(AxisFrame::flat(["North"])),
GridUpdate::ColFrame(AxisFrame::flat(["Revenue"])),
GridUpdate::Cells(vec![((0, 0), cell)]),
] {
state.fold(&Command::Project(Projection::Grid(cmd)));
}
}
/// BUG (improvise-3uzg) — see `context/test-evidence/clients.md`.
#[test]
fn an_awaited_seed_waits_for_the_answer_and_never_overwrites_typing() {
let mut state = ClientState::default();
answered_with(&mut state, "2");
state.fold_outgoing(&write_then_awaiting_editor());
assert_eq!(state.view.buffers.get("edit").map(String::as_str), Some(""));
answered_with(&mut state, "3");
assert_eq!(
state.view.buffers.get("edit").map(String::as_str),
Some(""),
"seeded while the write was still unanswered"
);
state.accepted();
assert_eq!(
state.view.buffers.get("edit").map(String::as_str),
Some("3"),
"the answered editor must open on the host's grid"
);
assert_eq!(
state.view.opened_editor,
Some(crate::OpenedEditor::on("3".into()))
);
let mut typed_ahead = ClientState::default();
answered_with(&mut typed_ahead, "2");
typed_ahead.fold_outgoing(&write_then_awaiting_editor());
typed_ahead.fold_outgoing(&[
Command::SetBuffer {
name: "edit".into(),
value: "5".into(),
},
Command::SetOpenedEditor(crate::resolve::edited(
typed_ahead.view.opened_editor.as_ref(),
"edit",
)),
]);
answered_with(&mut typed_ahead, "3");
typed_ahead.accepted();
assert_eq!(
typed_ahead.view.buffers.get("edit").map(String::as_str),
Some("5"),
"a seed arriving after the user typed must not overwrite the typing"
);
}
/// BUG (improvise-4cb5) — see `context/test-evidence/clients.md`.
#[test]
fn an_awaited_seed_arriving_after_escape_leaves_the_buffer_empty() {
let mut state = ClientState::default();
answered_with(&mut state, "2");
state.fold_outgoing(&write_then_awaiting_editor());
state.fold_outgoing(&[
Command::SetBuffer {
name: "edit".into(),
value: String::new(),
},
Command::ChangeMode(crate::AppMode::Normal),
]);
answered_with(&mut state, "3");
state.accepted();
assert_eq!(state.view.mode, crate::AppMode::Normal);
assert_eq!(
state.view.buffers.get("edit").map(String::as_str),
Some(""),
"the late seed was written into the buffer of an editor Esc had closed"
);
}
/// A frame the host does not answer must not consume an answer, or the
/// queue addresses the wrong frame. Evidence file.
#[test]
crates/improvise-protocol/src/help.rs
+13 -2
View File
@@ -335,6 +335,7 @@ mod key_row {
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct KeyRow {
keys: Vec<String>,
command: &'static str,
description: &'static str,
}
@@ -343,6 +344,12 @@ mod key_row {
&self.keys
}
/// The command the keys run, kept so a front end can ask whether it
/// can run it (improvise-uln5).
pub fn command(&self) -> &'static str {
self.command
}
pub fn description(&self) -> &'static str {
self.description
}
@@ -351,7 +358,7 @@ mod key_row {
/// advertises a key that does not exist.
fn looked_up(
mode: &ModeKey,
command: &str,
command: &'static str,
args: Option<&[String]>,
description: &'static str,
) -> Option<Self> {
@@ -360,7 +367,11 @@ mod key_row {
.map(|(c, m)| key_label(c, *m))
.collect();
keys.extend(prefixed_labels(mode, command, args));
(!keys.is_empty()).then_some(Self { keys, description })
(!keys.is_empty()).then_some(Self {
keys,
command,
description,
})
}
}
crates/improvise-protocol/src/keymap.rs
+11 -4
View File
@@ -347,6 +347,13 @@ impl TextEntry {
self
}
/// Bind `key` to a commit that clears the buffer **itself**, first, because
/// it reopens an editor a trailing clear would blank (improvise-fy9o).
pub fn commit_reopening(mut self, key: KeyCode, cmd: &'static str, args: Vec<String>) -> Self {
self.km.bind_args(key, KeyModifiers::NONE, cmd, args);
self
}
/// Replace a spine binding — used only by CommandMode, whose Backspace
/// exits the mode on an empty buffer instead of popping a character.
pub fn rebind(mut self, key: KeyCode, cmd: &'static str) -> Self {
@@ -853,8 +860,8 @@ impl Keymaps {
// edit-mode as an argument, so the command stays mode-agnostic. ──
let ed = Arc::new(
TextEntry::new(&AppMode::editing(), "normal", global.clone())
.commit(KeyCode::Enter, "commit-cell-edit", vec!["editing".into()])
.commit(
.commit_reopening(KeyCode::Enter, "commit-cell-edit", vec!["editing".into()])
.commit_reopening(
KeyCode::Tab,
"commit-and-advance-right",
vec!["editing".into()],
@@ -869,12 +876,12 @@ impl Keymaps {
ModeKey::RecordsEditing,
Arc::new(
TextEntry::new(&AppMode::records_editing(), "records-normal", ed)
.commit(
.commit_reopening(
KeyCode::Enter,
"commit-cell-edit",
vec!["records-editing".into()],
)
.commit(
.commit_reopening(
KeyCode::Tab,
"commit-and-advance-right",
vec!["records-editing".into()],
crates/improvise-protocol/src/resolve.rs
+317 -72
View File
@@ -359,6 +359,34 @@ fn not_yet(what: &str) -> String {
format!("{what} is not available in the browser yet")
}
/// The command names this client has no arm for, so their keys go upstream to
/// a host that answers nothing (gotcha 47). The one list: see clients.md.
pub const NEEDS_THE_HOST: &[&str] = &[
"enter-advance",
"exit-search-mode",
"force-quit",
"hide-selected-row-item",
"search",
"search-navigate",
"toggle-group-under-cursor",
];
/// Whether a key running `name` does nothing in this client.
pub fn needs_the_host(name: &str) -> bool {
NEEDS_THE_HOST.contains(&name)
}
/// Whether `:` opens a command line this client can run; it cannot, so an
/// ex-command it advertises is unreachable (improvise-uln5).
pub fn runs_the_command_line() -> bool {
AppMode::from_name("command").is_some_and(|mode| stranding(&mode).is_none())
}
/// A help description, marked as naming something this client cannot do.
pub fn not_here(description: &str) -> String {
format!("{description} (not in the browser yet)")
}
/// The subset of command names this client can turn into `Command`s; `None`
/// means "the host's". Each arm mirrors its `Cmd` line for line — clients.md.
fn composed(
@@ -781,14 +809,16 @@ fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option<Vec<Command>> {
match cell.id.as_ref()? {
// `edit_text`, never `text`: the drawn text has been through
// `format_f64`. `tests/lockstep.rs` found exactly that.
CellId::Record(_) => Some(opened_on(cell.edit_text.clone().unwrap_or_default(), mode)),
CellId::Record(_) => Some(opened_on(
state,
cell.edit_text.clone().unwrap_or_default(),
mode,
)),
CellId::Aggregate(_) => Some(vec![Command::SetStatus(format!(
"an aggregate is edited by drilling in — {}",
not_yet("drill")
))]),
CellId::Derived { target } => Some(vec![Command::SetStatus(format!(
"{target} is computed by a formula — edit the formula, not the cell"
))]),
CellId::Derived { target } => Some(vec![Command::SetStatus(derived_decline(target))]),
// Both creating columns are records-only, so the guard above takes
// them; declining rather than `None` keeps that a door, not a hole.
CellId::Create(_) => Some(vec![Command::SetStatus(not_yet("creating a column"))]),
@@ -796,9 +826,18 @@ fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option<Vec<Command>> {
}
}
/// Open the cell editor on `seed`: the buffer, what it captured, then the mode.
/// `EditOrDrill`'s and `EnterEditAtCursor`'s shared shape (improvise-ixxx).
fn opened_on(seed: String, mode: AppMode) -> Vec<Command> {
/// Open the cell editor on `seed`, or await it while any write is unanswered: the
/// cache predates that write (improvise-mdfc/p6v6, `protocol.md` §3).
fn opened_on(state: &ClientState, seed: String, mode: AppMode) -> Vec<Command> {
match state.in_flight.iter().any(|sent| sent.awaited) {
true => awaiting_seed(mode),
false => seeded(seed, mode),
}
}
/// The buffer, what the editor captured, then the mode: `EditOrDrill`'s and
/// `EnterEditAtCursor`'s shared shape (improvise-ixxx).
fn seeded(seed: String, mode: AppMode) -> Vec<Command> {
vec![
Command::SetBuffer {
name: "edit".to_string(),
@@ -917,10 +956,20 @@ fn commit(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option<V
}
let mut commands = written(state, id, &text)?;
commands.push(Command::ChangeMode(AppMode::Normal));
commands.extend(advanced(state, edit_mode, advance)?);
commands.extend(cleared(&edit_mode));
let writes = !untouched(state.view.opened_editor.as_ref(), &text);
commands.extend(advanced(state, edit_mode, advance, writes)?);
Some(commands)
}
/// The commit's own clear of `edit_mode`'s buffer, at its head: the keymap
/// welds none to these keys, so a re-seed survives (improvise-fy9o).
fn cleared(edit_mode: &AppMode) -> Option<Command> {
edit_mode
.minibuffer()
.map(|mb| write_buffer(&mb.buffer_key, String::new()))
}
/// The write a commit makes: none when the buffer is the seed the editor
/// captured as it opened, and only the write is skipped (improvise-ixxx).
fn written(state: &ClientState, id: &CellId, text: &str) -> Option<Vec<Command>> {
@@ -945,6 +994,7 @@ fn create_column(
// `exit_mode_for`: the tidy-up goes FIRST, where an aborting
// `AddCategory` cannot reach it (gotcha 21).
let mut commands = vec![Command::ChangeMode(AppMode::Normal)];
commands.extend(cleared(&edit_mode));
let name = text.trim();
if !name.is_empty() {
commands.push(model(
@@ -965,13 +1015,18 @@ fn create_column(
if matches!(kind, CreateKind::Category) {
return Some(commands);
}
commands.extend(advanced(state, edit_mode, advance)?);
commands.extend(advanced(state, edit_mode, advance, !name.is_empty())?);
Some(commands)
}
/// The advance and the re-entry `CommitAndAdvance` ends in. `None` where the
/// host would emit `AddRecordRow`, which this client cannot name (gotcha 1).
fn advanced(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option<Vec<Command>> {
fn advanced(
state: &ClientState,
edit_mode: AppMode,
advance: Advance,
writes: bool,
) -> Option<Vec<Command>> {
let GridExtent { rows, cols } = state.grid.extent;
let row_max = rows.saturating_sub(1);
let col_max = cols.saturating_sub(1);
@@ -993,12 +1048,25 @@ fn advanced(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option
return Some(Vec::new());
};
let mut commands = scrolled_to(state, nr, nc);
// `EnterEditAtCursor`: seed from where we landed, then change mode. The
// new cell is a different cell, so a stale cache is correct here.
commands.extend(opened_on(seed_at(state, nr, nc), edit_mode));
// This frame's own write makes the cache stale too; `opened_on` answers for
// earlier ones (improvise-3uzg/mdfc/p6v6, `protocol.md` §3).
commands.extend(reopened_at(state, nr, nc, edit_mode, writes));
Some(commands)
}
/// Open the cell editor with its seed still to come: the client settles it from
/// the host's answer, once nothing this client sent is unanswered (improvise-3uzg).
fn awaiting_seed(mode: AppMode) -> Vec<Command> {
vec![
Command::SetBuffer {
name: EDIT_BUFFER.to_string(),
value: String::new(),
},
Command::SetOpenedEditor(Some(OpenedEditor::awaiting())),
Command::ChangeMode(mode),
]
}
/// The surrogate naming the record under the cursor, or `None` when the
/// cursor is not on a writable cell.
fn record_at_cursor(state: &ClientState) -> Option<crate::RecordHash> {
@@ -1018,20 +1086,47 @@ pub(crate) fn edit_seed(state: &ClientState) -> Option<String> {
/// The display text at an absolute grid position, as the cache holds it.
fn seed_at(state: &ClientState, row: usize, col: usize) -> String {
let cursor = state.view.cursor();
let Some(r) = row.checked_sub(cursor.row_offset) else {
return String::new();
};
let Some(c) = col.checked_sub(cursor.col_offset) else {
return String::new();
};
state
.grid
.cell(r, c)
cell_at(state, row, col)
.and_then(|cell| cell.edit_text.clone())
.unwrap_or_default()
}
/// The projected cell at an absolute grid position, `None` outside the window.
fn cell_at(state: &ClientState, row: usize, col: usize) -> Option<&crate::CellDisplay> {
let cursor = state.view.cursor();
let r = row.checked_sub(cursor.row_offset)?;
let c = col.checked_sub(cursor.col_offset)?;
state.grid.cell(r, c)
}
/// `EnterEditAtCursor`: re-open the editor where the advance landed, unless `i`
/// would decline that cell. The host asks `uneditable`; this asks the same id.
fn reopened_at(
state: &ClientState,
row: usize,
col: usize,
mode: AppMode,
writes: bool,
) -> Vec<Command> {
match cell_at(state, row, col).and_then(|cell| cell.id.as_ref()) {
Some(CellId::Aggregate(_)) => vec![Command::SetStatus(AGGREGATE_DECLINE.to_string())],
Some(CellId::Derived { target }) => vec![Command::SetStatus(derived_decline(target))],
Some(CellId::Record(_) | CellId::Create(_) | CellId::Coordinate) | None => match writes {
true => awaiting_seed(mode),
false => opened_on(state, seed_at(state, row, col), mode),
},
}
}
/// What the host says of an aggregate, the fold of a collapsed group included.
const AGGREGATE_DECLINE: &str =
"Cannot write to an aggregate — it combines several records, so there is no one cell to write";
/// What both ends say of a formula's result: the host's `derived_refusal`.
fn derived_decline(target: &str) -> String {
format!("{target} is computed by a formula — edit the formula, not the cell")
}
/// What a cell editor captured as it **opened**, so its commit is judged
/// against that and never a seed read again at commit (improvise-ixxx).
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
@@ -1039,6 +1134,9 @@ pub struct OpenedEditor {
pub seed: String,
/// Whether the user has edited the buffer since it opened (improvise-2054).
pub touched: bool,
/// The seed is still to come from the host's answer (improvise-3uzg). Not
/// `serde(default)`: `false` is a claim — `protocol.md` §3, improvise-mgis.
pub awaiting: bool,
}
impl OpenedEditor {
@@ -1047,6 +1145,16 @@ impl OpenedEditor {
Self {
seed,
touched: false,
awaiting: false,
}
}
/// An editor opened before its seed is known: empty until it settles.
pub fn awaiting() -> Self {
Self {
seed: String::new(),
touched: false,
awaiting: true,
}
}
}
@@ -2402,7 +2510,12 @@ mod tests {
}
}
}
for (where_, got) in report.into_iter().filter(|(_, got)| *got != Heard::Acts) {
// A black hole the browser marks is the next test's, by NEEDS_THE_HOST.
let marked = |got: &Heard| *got == Heard::Silent && needs_the_host(command);
for (where_, got) in report
.into_iter()
.filter(|(_, got)| *got != Heard::Acts && !marked(got))
{
short
.entry((where_, command, got))
.or_default()
@@ -2426,14 +2539,6 @@ mod tests {
"COMMAND: Enter execute-command Silent",
"EXPORT: Enter commit-export Silent",
"SAVE AS: Enter commit-save Silent",
"NORMAL SEARCH: Enter/Esc exit-search-mode Silent",
// Commands the browser has no arm for: upstream, so a black hole
// (improvise-uln5). Each is also in the host-only list below.
"NORMAL: Enter enter-advance Silent",
"NORMAL: H hide-selected-row-item Silent",
"NORMAL: / search Silent",
"NORMAL: n search-navigate Silent",
"NORMAL: z toggle-group-under-cursor Silent",
// Honestly unavailable, and saying so in `declined`'s words.
"FORMULAS: Del/d delete-formula-at-cursor Declines",
"NORMAL: > drill-into-cell Declines",
@@ -2667,41 +2772,93 @@ mod tests {
/// closing one of these gaps has to come here and say so. Evidence file.
#[test]
fn the_command_names_that_still_need_the_host_are_these() {
let mut state = in_mode(AppMode::Normal);
state.grid.extent = GridExtent { rows: 4, cols: 4 };
state.window = (4, 4);
for (key, name) in [
// The three prefixes left with improvise-hs0u: arming is a
// `Command`, and the second key resolves in the sub-keymap.
(ev(KeyCode::Char('H')), "hide-selected-row-item"),
(ev(KeyCode::Char('z')), "toggle-group-under-cursor"),
// `>`, `R`, `P`, Ctrl-W and Ctrl-E are NOT here any more: each
// answers with a status of its own (improvise-gd28, improvise-rmvx).
// `<` left too, and further: since improvise-cfly it composes a
// real `Command::ViewBack` rather than a refusal.
(ev(KeyCode::Char('/')), "search"),
(ev(KeyCode::Char('n')), "search-navigate"),
// `N` left this list with improvise-koh6: both its arms are
// answered here, one of them out loud. See clients.md.
(ev(KeyCode::Enter), "enter-advance"),
// Ctrl-S left this list with improvise-jz6w: it is a `Request`,
// not a `Command`, and the host has answered it since 368h.
(
KeyEvent::new(KeyCode::Char('q'), KeyModifiers::CONTROL),
"force-quit",
),
] {
assert_eq!(
resolve(&state, key),
KeyOutcome::Upstream(key),
"{name} ({:?}) was resolved locally; if that is deliberate, \
this list is where it stops being a gap",
key.code
// `N` left with improvise-koh6, Ctrl-S with jz6w, and `>`, `R`, `P`,
// Ctrl-W, Ctrl-E and `<` with gd28, rmvx and cfly: see clients.md.
for name in NEEDS_THE_HOST {
let mut sequences = Vec::new();
for (mode, search) in [(AppMode::Normal, false), (AppMode::Normal, true)] {
let key = ModeKey::from_app_mode(&mode, search).expect("a keyed mode");
let keymap = Keymaps::shared().get(&key).expect("a keymap");
for sequence in sequences_running(keymap, name, None) {
sequences.push((mode.clone(), search, sequence));
}
}
assert!(
!sequences.is_empty(),
"{name} is bound nowhere this test looks"
);
for (mode, search, sequence) in sequences {
let mut state = in_mode(mode);
state.view.search_mode = search;
state.grid.extent = GridExtent { rows: 4, cols: 4 };
state.window = (4, 4);
let (key, prefix) = sequence.split_last().expect("a key");
for armed in prefix {
if let KeyOutcome::Local(commands) = resolve(&state, *armed) {
commands.iter().for_each(|c| state.fold(c));
}
}
let key = *key;
assert_eq!(
resolve(&state, key),
KeyOutcome::Upstream(key),
"{name} ({:?}) was resolved locally; if that is deliberate, it \
leaves NEEDS_THE_HOST, and the browser stops marking it",
key.code
);
}
}
}
/// improvise-uln5: the help rows a browser cannot run are exactly the ones
/// it marks, and no hint names one. See `context/test-evidence/clients.md`.
#[test]
fn every_help_row_the_browser_cannot_run_is_one_it_marks() {
for mode in AppMode::all() {
for (command, _, _) in crate::help::advertised(&mode) {
assert!(
!needs_the_host(command),
"the {mode:?} hint line advertises {command}, which a browser \
cannot run and which the hint line never marks"
);
}
}
let mut silent = Vec::new();
let mut marked = Vec::new();
for (key, command, args, _) in crate::help::help_bindings() {
let (mode, search) = match &key {
ModeKey::SearchMode => (AppMode::Normal, true),
key => (
AppMode::all()
.into_iter()
.find(|m| ModeKey::from_app_mode(m, false).as_ref() == Some(key))
.expect("a help row's mode is reachable"),
false,
),
};
let keymap = Keymaps::shared().get(&key).expect("a keymap");
for sequence in sequences_running(keymap, command, args.as_deref()) {
let mut state = furnished(mode.clone());
state.view.search_mode = search;
if press(state, &sequence) == Heard::Silent {
silent.push(command);
}
}
if needs_the_host(command) {
marked.push(command);
}
}
for list in [&mut silent, &mut marked] {
list.sort();
list.dedup();
}
assert_eq!(
silent, marked,
"left: help rows whose keys do nothing in a browser; right: the rows \
it marks as needing the terminal. They must be the same rows."
);
}
/// improvise-jz6w: the outcome a resolver had no way to name. See
/// `context/test-evidence/clients.md`.
#[test]
@@ -3400,18 +3557,31 @@ mod tests {
}
);
assert_eq!(commands[1], Command::ChangeMode(AppMode::Normal));
assert_eq!(commands[2], Command::SetSelected { row: 1, col: 0 });
assert!(
commands.contains(&Command::ChangeMode(AppMode::editing())),
"the advance re-enters editing on the cell it landed on: {commands:?}"
);
// The commit owns its clear, at the head (improvise-fy9o).
assert_eq!(
commands.last(),
Some(&Command::SetBuffer {
commands[2],
Command::SetBuffer {
name: "edit".into(),
value: String::new(),
}),
"the keymap welds `clear-buffer` to every commit: {commands:?}"
}
);
assert_eq!(commands[3], Command::SetSelected { row: 1, col: 0 });
assert_eq!(
commands.last(),
Some(&Command::ChangeMode(AppMode::editing())),
"the advance re-enters editing on the cell it landed on: {commands:?}"
);
assert!(
commands.contains(&Command::SetOpenedEditor(Some(OpenedEditor::awaiting()))),
"after a write the reopened editor awaits the host's seed: {commands:?}"
);
for command in &commands {
client.fold(command);
}
assert_eq!(
client.view.buffers.get("edit").map(String::as_str),
Some("900"),
"an answered client seeds the reopened editor from the cell it landed on"
);
}
@@ -3454,6 +3624,51 @@ mod tests {
commands
}
/// Tab in an editor lands one column right, off a centred cursor that no
/// edge clamps (improvise-rq3f). Evidence: `context/test-evidence/clients.md`.
#[test]
fn tab_in_an_editor_commits_and_lands_one_column_right() {
let mut client = centred_on(record_cell());
press_folded(&mut client, KeyCode::Char('i'));
assert!(client.view.mode.is_editing(), "setup: the editor opened");
let commands = press_folded(&mut client, KeyCode::Tab);
assert!(
commands.contains(&Command::SetSelected { row: 1, col: 2 }),
"Tab must advance right from (1, 1): {commands:?}"
);
assert_eq!(client.view.cursor().selected, (1, 2));
assert!(
client.view.mode.is_editing(),
"and re-enter the editor there"
);
}
/// Right on the last tile stays on it, and still emits, as `MoveTileCursor`
/// does (improvise-rq3f). Evidence: `context/test-evidence/clients.md`.
#[test]
fn the_tile_cursor_stops_on_the_last_tile() {
let mut client = furnished(AppMode::TileSelect);
let last = client.tiles.tiles.len() - 1;
assert_eq!(
client.view.tile_cat_idx,
last - 1,
"setup: one short of the end"
);
assert_eq!(
press_folded(&mut client, KeyCode::Right),
vec![Command::SetTileCatIdx(last)],
"the control: a move that is not clamped"
);
for code in [KeyCode::Right, KeyCode::Char('l')] {
assert_eq!(
press_folded(&mut client, code),
vec![Command::SetTileCatIdx(last)],
"{code:?} past the last tile must clamp to it"
);
}
assert_eq!(client.view.tile_cat_idx, last);
}
/// BUG (improvise-ixxx) — see `context/test-evidence/clients.md`.
#[test]
fn an_untouched_browser_commit_writes_nothing_and_still_advances() {
@@ -4006,3 +4221,33 @@ mod tests {
);
}
}
#[cfg(test)]
mod opened_editor_wire_tests {
use super::OpenedEditor;
use crate::{ClientFrame, Command};
/// improvise-mgis: `awaiting: false` is the claim that the seed is settled,
/// so a frame that dropped it is refused. `context/test-evidence/clients.md`.
#[test]
fn a_frame_whose_editor_lost_its_awaiting_flag_does_not_parse() {
let frame = ClientFrame::Commands(vec![Command::SetOpenedEditor(Some(
OpenedEditor::awaiting(),
))]);
let mut json = serde_json::to_value(&frame).expect("a ClientFrame serializes");
let opened = json
.pointer_mut("/Commands/0/SetOpenedEditor")
.and_then(serde_json::Value::as_object_mut)
.expect("the editor is where this test expects it");
assert!(
opened.remove("awaiting").is_some() && opened.contains_key("seed"),
"the field must have been there, and the rest of the editor must remain"
);
let err = serde_json::from_value::<ClientFrame>(json)
.expect_err("an editor that states no awaiting flag states no claim");
assert!(
err.to_string().contains("missing field `awaiting`"),
"it must fail as the MISSING FIELD, not as broken JSON: {err}"
);
}
}
crates/improvise-wasm-client/src/render.rs
+72 -13
View File
@@ -2,7 +2,7 @@
//! already made. The four silent ones are in `context/code/clients.md`.
use improvise_protocol::viewmodel::{CatTreeEntry, flatten_cat_tree};
use improvise_protocol::{AxisFrame, CellDisplay, CellId, FrameEntry};
use improvise_protocol::{AxisFrame, CellDisplay, CellId, FrameEntry, resolve};
use serde::Serialize;
use crate::{Client, ClientState};
@@ -423,22 +423,37 @@ impl RenderHelpRow {
L::Heading(t) => prose("heading", t),
L::Text(t) => prose("text", t),
L::Accent(t) => prose("accent", t),
L::Keys(row) => Self {
keys: row.keys().join(" "),
description: row.description().to_string(),
kind: "keys",
},
L::Keys(row) => Self::row(
row.keys().join(" "),
row.description(),
!resolve::needs_the_host(row.command()),
false,
),
L::Literal {
keys,
description,
dim,
} => Self {
keys: (*keys).to_string(),
description: description.to_string(),
kind: match dim {
true => "dim",
false => "keys",
},
} => Self::row(
(*keys).to_string(),
description,
resolve::runs_the_command_line(),
*dim,
),
}
}
/// A keys row. One this client cannot run is drawn dim and says so, since a
/// dead key followed by live ones edits the grid (improvise-uln5, koh6).
fn row(keys: String, description: &str, runs: bool, dim: bool) -> Self {
Self {
keys,
description: match runs {
true => description.to_string(),
false => resolve::not_here(description),
},
kind: match dim || !runs {
true => "dim",
false => "keys",
},
}
}
@@ -579,6 +594,50 @@ mod tests {
FrameEntry::Item(vec![label.into()])
}
fn help_page(page: usize) -> Vec<RenderHelpRow> {
let view = ViewState {
mode: improvise_protocol::AppMode::Help,
help_page: page,
..ViewState::default()
};
help_rows(&view)
}
/// improvise-uln5: a row whose key this client sends to a host that answers
/// nothing is drawn marked. Evidence: `context/test-evidence/clients.md`.
#[test]
fn a_help_row_the_browser_cannot_run_is_drawn_marked() {
let rows = help_page(1);
let row = |prefix: &str| {
let found = rows.iter().find(|r| r.description.starts_with(prefix));
found.unwrap_or_else(|| panic!("no Navigation row reads {prefix:?}"))
};
let search = row("Start search");
assert_eq!(search.kind, "dim", "{search:?}");
assert!(
search.description.contains("not in the browser"),
"{search:?}"
);
let control = row("Jump to last row");
assert_eq!(
control.kind, "keys",
"a row the browser runs is drawn plainly"
);
assert_eq!(control.description, "Jump to last row");
let commands = help_page(4);
let ex: Vec<_> = commands
.iter()
.filter(|r| r.keys.trim_start().starts_with(':'))
.collect();
assert!(
!ex.is_empty(),
"precondition: the Commands page lists ex-commands"
);
for row in ex {
assert!(row.description.contains("not in the browser"), "{row:?}");
}
}
/// A run is opened by a HEADER, not by a name — improvise-h0iv, and both
/// ends now ask the one rule. See the evidence file.
#[test]
crates/improvise-wasm-client/tests/lockstep.rs
+491 -26
View File
@@ -1727,6 +1727,45 @@ fn a_locally_committed_creating_column_agrees_with_the_host() {
}
}
/// improvise-5wai: `+cat` reopens nothing, so its commit's own clear is all
/// that empties the buffer, on both ends. Evidence file.
#[test]
fn a_category_committed_at_plus_cat_leaves_no_buffer_behind_on_both_ends() {
let (mut app, mut twin) = records_pair();
let client = furnished_client(&app, (12, 12));
let at = creating_cell(&client, improvise_protocol::CreateKind::Category);
let mut client = client;
typing_at(&mut app, &mut twin, &mut client, at, "Season");
client = furnished_client(&app, (12, 12));
client.view = app.view_state.clone();
assert_eq!(
read_edit(&client.view.buffers),
"Season",
"nothing was typed, so this proves nothing"
);
both_ends(&mut app, &mut twin, &mut client, "Enter");
assert!(
document_text(&app).contains("Season"),
"the commit did not land"
);
assert_eq!(
read_edit(&app.view_state.buffers),
"",
"the host left the name behind"
);
assert_eq!(
read_edit(&client.view.buffers),
"",
"the browser left the name behind"
);
}
fn read_edit(buffers: &std::collections::HashMap<String, String>) -> &str {
buffers.get("edit").map(String::as_str).unwrap_or("")
}
/// The twin of the commit test above, for the door rather than the exit
/// (improvise-bd9o). Evidence file.
#[test]
@@ -2091,36 +2130,83 @@ fn p_onto_a_formula_owned_cell_declines_on_both_ends() {
/// improvise-gw18/l4mp: `p` and `x` on a collapsed group's fold, off the origin.
#[test]
fn a_write_onto_a_fold_declines_on_both_ends() {
let fold_host = || {
let mut wb = Workbook::new("Test");
wb.add_category("Month").unwrap();
for (month, quarter) in [("Jan", "Q1"), ("Feb", "Q1"), ("Apr", "Q2"), ("May", "Q2")] {
wb.add_item_in_group("Month", month, quarter).unwrap();
}
wb.add_item("_Measure", "Revenue").unwrap();
wb.add_item("_Measure", "Cost").unwrap();
for month in ["Jan", "Feb", "Apr", "May"] {
for measure in ["Revenue", "Cost"] {
let key = improvise_command::model::cell::CellKey::new(vec![
("Month".to_string(), month.to_string()),
("_Measure".to_string(), measure.to_string()),
]);
wb.model
.set_cell(key, improvise_protocol::CellValue::Number(3.0));
}
}
wb.active_view_mut().set_axis("Month", Axis::Row);
wb.active_view_mut().set_axis("_Measure", Axis::Column);
wb.active_view_mut().toggle_group_collapse("Month", "Q2");
let mut app = App::new(wb, None);
app.rebuild_layout();
assert_eq!(app.layout.row_label(2), "Q2", "setup: the fold is row 2");
app
};
declines_on_both_ends(fold_host(), (2, 1), 'p');
declines_on_both_ends(fold_host(), (2, 1), 'x');
}
/// Months grouped into quarters on Row, two measures on Column, Q2 collapsed:
/// rows `[Jan, Feb, Q2]`, so the fold is row 2 and off the origin.
fn fold_host() -> App {
let mut wb = Workbook::new("Test");
wb.add_category("Month").unwrap();
for (month, quarter) in [("Jan", "Q1"), ("Feb", "Q1"), ("Apr", "Q2"), ("May", "Q2")] {
wb.add_item_in_group("Month", month, quarter).unwrap();
}
wb.add_item("_Measure", "Revenue").unwrap();
wb.add_item("_Measure", "Cost").unwrap();
for month in ["Jan", "Feb", "Apr", "May"] {
for measure in ["Revenue", "Cost"] {
let key = improvise_command::model::cell::CellKey::new(vec![
("Month".to_string(), month.to_string()),
("_Measure".to_string(), measure.to_string()),
]);
wb.model
.set_cell(key, improvise_protocol::CellValue::Number(3.0));
}
}
wb.active_view_mut().set_axis("Month", Axis::Row);
wb.active_view_mut().set_axis("_Measure", Axis::Column);
wb.active_view_mut().toggle_group_collapse("Month", "Q2");
let mut app = App::new(wb, None);
app.rebuild_layout();
assert_eq!(app.layout.row_label(2), "Q2", "setup: the fold is row 2");
app
}
/// improvise-enz6: a commit that advances onto a cell `i` declines re-opens no
/// editor there, on either end. See `context/test-evidence/clients.md`.
#[test]
fn advancing_the_editor_onto_a_cell_i_declines_leaves_both_ends_out_of_it() {
let cases = [
(fold_host(), (0, 1), "Enter", (2, 1)),
(yank_host(), (1, 1), "Tab", (1, 3)),
];
for (mut app, at, key, lands) in cases {
app.cursor_mut().selected = at;
let mut client = client_ready_to_edit(&app, (6, 6));
for (step, key) in ["i", key, key].into_iter().enumerate() {
let event = BrowserKey::new(key).to_key_event().unwrap();
let KeyOutcome::Local(cmds) = resolve(&client, event) else {
panic!("{key}: the client must answer this key locally");
};
for cmd in &cmds {
client.fold(cmd);
}
reduce_key(&mut app, event);
assert_eq!(
format!("{:?}", client.view.mode),
format!("{:?}", app.view_state.mode),
"step {step} ({key}): client and host disagree about the mode"
);
assert_eq!(client.view.cursor(), app.view_state.cursor(), "step {step}");
assert_eq!(client.view.buffers, app.view_state.buffers, "step {step}");
}
assert_eq!(
app.view_state.cursor().selected,
lands,
"setup: {key} landed"
);
assert!(
!app.view_state.mode.is_editing() && !client.view.mode.is_editing(),
"{key}: an editor is open on a cell i declines"
);
assert!(
!client.view.status_msg.is_empty(),
"{key}: the browser declined silently"
);
}
}
/// Two regions by two measures, `value` at (South, Revenue): row 1, column 1,
/// so "the cell under the cursor" is not "the first cell".
fn precise_host(value: improvise_protocol::CellValue) -> App {
@@ -2183,6 +2269,179 @@ fn an_untouched_browser_edit_of_a_misread_value_writes_nothing() {
}
}
/// BUG (improvise-fy9o) — see `context/test-evidence/clients.md`.
#[test]
fn enter_enter_leaves_every_cell_unchanged_on_both_ends() {
let every_cell = |app: &App| -> Vec<Option<improvise_protocol::CellValue>> {
[
("North", "Cost"),
("North", "Revenue"),
("South", "Cost"),
("South", "Revenue"),
]
.iter()
.map(|(r, m)| {
app.model_state
.workbook
.model
.get_cell(&region_measure(r, m))
.cloned()
})
.collect()
};
let mut client_host = precise_host(improvise_protocol::CellValue::Number(4.0));
client_host.cursor_mut().selected = (1, 0);
let mut key_host = precise_host(improvise_protocol::CellValue::Number(4.0));
key_host.cursor_mut().selected = (1, 0);
let before = every_cell(&client_host);
let mut client = client_ready_to_edit(&client_host, (2, 2));
for key in ["i", "Enter", "Enter"] {
let event = BrowserKey::new(key).to_key_event().unwrap();
let KeyOutcome::Local(cmds) = resolve(&client, event) else {
panic!("{key}: the client must answer this key locally");
};
for cmd in &cmds {
client.fold(cmd);
}
apply_on(&mut client_host, &cmds);
reduce_key(&mut key_host, event);
assert_eq!(
client.view.buffers, key_host.view_state.buffers,
"{key}: client and host disagree about the buffers"
);
}
assert_eq!(
every_cell(&client_host),
before,
"the browser's Enter, Enter wrote"
);
assert_eq!(
every_cell(&key_host),
before,
"the host's Enter, Enter wrote"
);
}
/// A commit that reopens nothing still leaves no text behind, on either end:
/// the clear moved from the keymap into the commit (improvise-fy9o).
#[test]
fn a_commit_at_the_corner_leaves_no_buffer_behind_on_both_ends() {
let mut client_host = precise_host(improvise_protocol::CellValue::Number(4.0));
let mut key_host = precise_host(improvise_protocol::CellValue::Number(4.0));
assert_eq!(
key_host.cursor().selected,
(1, 1),
"the fixture is not at the corner"
);
let mut client = client_ready_to_edit(&client_host, (2, 2));
for key in ["i", "9", "Enter"] {
let event = BrowserKey::new(key).to_key_event().unwrap();
let KeyOutcome::Local(cmds) = resolve(&client, event) else {
panic!("{key}: the client must answer this key locally");
};
for cmd in &cmds {
client.fold(cmd);
}
apply_on(&mut client_host, &cmds);
reduce_key(&mut key_host, event);
}
assert!(
!key_host.view_state.mode.is_editing(),
"the corner must not reopen"
);
for (end, buffers) in [
("browser", &client.view.buffers),
("host", &key_host.view_state.buffers),
] {
assert_eq!(
buffers.get("edit").map(String::as_str),
Some(""),
"{end}: the committed text was left in the buffer"
);
}
}
/// BUG (improvise-3tt4) — see `context/test-evidence/clients.md`.
#[test]
fn tab_at_the_rightmost_column_reopens_on_the_value_just_written() {
let mut client_host = precise_host(improvise_protocol::CellValue::Number(4.0));
client_host.cursor_mut().selected = (0, 1);
let mut key_host = precise_host(improvise_protocol::CellValue::Number(4.0));
key_host.cursor_mut().selected = (0, 1);
let mut client = client_ready_to_edit(&client_host, (2, 2));
for key in ["i", "9", "Tab", "5", "Enter"] {
let event = BrowserKey::new(key).to_key_event().unwrap();
sent_and_answered(&mut client, &mut client_host, event, (2, 2));
reduce_key(&mut key_host, event);
assert_eq!(
client.view.buffers, key_host.view_state.buffers,
"{key}: client and host disagree about the buffers"
);
}
for (end, app) in [("browser", &client_host), ("host", &key_host)] {
assert_eq!(
app.model_state
.workbook
.model
.get_cell(&region_measure("North", "Revenue")),
Some(&improvise_protocol::CellValue::Number(295.0)),
"{end}: the reopened editor did not extend the value Tab had just written"
);
}
}
/// The reopened seed is the STORED value's edit text, not the typed string:
/// `007` is stored as 7 on both ends (improvise-3tt4). Evidence file.
#[test]
fn tab_at_the_rightmost_column_reopens_on_the_stored_spelling() {
let mut client_host = precise_host(improvise_protocol::CellValue::Number(4.0));
client_host.cursor_mut().selected = (0, 1);
let mut key_host = precise_host(improvise_protocol::CellValue::Number(4.0));
key_host.cursor_mut().selected = (0, 1);
let mut client = client_ready_to_edit(&client_host, (2, 2));
for key in ["i", "Backspace", "0", "0", "7", "Tab"] {
let event = BrowserKey::new(key).to_key_event().unwrap();
sent_and_answered(&mut client, &mut client_host, event, (2, 2));
reduce_key(&mut key_host, event);
}
assert_eq!(
key_host.view_state.buffers.get("edit").map(String::as_str),
Some("7")
);
assert_eq!(client.view.buffers, key_host.view_state.buffers);
}
/// BUG (improvise-fy9o) — see `context/test-evidence/clients.md`.
#[test]
fn typing_into_the_reopened_editor_extends_the_cell_on_both_ends() {
let mut client_host = precise_host(improvise_protocol::CellValue::Number(4.0));
client_host.cursor_mut().selected = (1, 0);
let mut key_host = precise_host(improvise_protocol::CellValue::Number(4.0));
key_host.cursor_mut().selected = (1, 0);
let mut client = client_ready_to_edit(&client_host, (2, 2));
for key in ["i", "Enter", "5", "Enter"] {
let event = BrowserKey::new(key).to_key_event().unwrap();
let KeyOutcome::Local(cmds) = resolve(&client, event) else {
panic!("{key}: the client must answer this key locally");
};
for cmd in &cmds {
client.fold(cmd);
}
apply_on(&mut client_host, &cmds);
reduce_key(&mut key_host, event);
}
for (end, app) in [("browser", &client_host), ("host", &key_host)] {
assert_eq!(
app.model_state
.workbook
.model
.get_cell(&region_measure("North", "Revenue")),
Some(&improvise_protocol::CellValue::Number(25.0)),
"{end}: the digit typed into the reopened editor did not extend its seed"
);
}
}
/// `i` then `Enter` on (South, Revenue) holding `value`, through the resolver
/// and a real host: the seed is the stored value's, and the cell is unchanged.
fn untouched_browser_edit(value: improvise_protocol::CellValue) {
@@ -2268,6 +2527,212 @@ fn every_projected_edit_text_commits_back_to_its_value() {
}
}
/// North/South/West = 1/2/3 on one measure, with `prune_empty` on: emptying a
/// row's only value takes the row away and renumbers the rest.
fn pruned_host() -> App {
let mut wb = Workbook::new("Test");
wb.add_category("Region").unwrap();
for region in ["North", "South", "West"] {
wb.add_item("Region", region).unwrap();
}
wb.add_item("_Measure", "Revenue").unwrap();
wb.active_view_mut().set_axis("Region", Axis::Row);
wb.active_view_mut().set_axis("_Measure", Axis::Column);
wb.active_view_mut().prune_empty = true;
for (region, value) in [("North", 1.0), ("South", 2.0), ("West", 3.0)] {
wb.model.set_cell(
region_measure(region, "Revenue"),
improvise_protocol::CellValue::Number(value),
);
}
let mut app = App::new(wb, None);
app.term_height = 3 + 8;
app.rebuild_layout();
app
}
/// One key the way a real browser sends it: folded as an outgoing frame, applied
/// by the host, then answered with the host's own projection and `Accepted`.
fn sent_and_answered(
client: &mut ClientState,
host: &mut App,
event: KeyEvent,
window: (usize, usize),
) {
let KeyOutcome::Local(cmds) = resolve(client, event) else {
panic!("{event:?}: the client must answer this key locally");
};
client.fold_outgoing(&cmds);
apply_on(host, &cmds);
client.grid = client_of(host, window).grid;
improvise_protocol::WidgetCache::apply(&mut client.basis, &published_basis(host, window));
client.accepted();
}
/// BUG (improvise-3uzg) — see `context/test-evidence/clients.md`.
#[test]
fn a_commit_that_prunes_its_row_reopens_on_the_row_that_moved_up() {
let window = (3, 3);
let mut client_host = pruned_host();
let mut key_host = pruned_host();
let mut client = client_ready_to_edit(&client_host, window);
for key in ["i", "Backspace", "Enter", "5", "Enter"] {
let event = BrowserKey::new(key).to_key_event().unwrap();
sent_and_answered(&mut client, &mut client_host, event, window);
reduce_key(&mut key_host, event);
assert_eq!(
client.view.buffers, key_host.view_state.buffers,
"{key}: client and host disagree about the buffers"
);
}
assert_eq!(
document_text(&client_host),
document_text(&key_host),
"the browser wrote a value built from another record's number"
);
assert!(
document_text(&key_host).contains("= 35"),
"setup: the host's reopened editor extends West's 3"
);
}
/// What the host owes a frame it has applied: its projection, its basis, and
/// `Accepted` when the frame was awaited. Held back so the next key overtakes it.
struct Unanswered {
grid: improvise_protocol::GridCache,
basis: improvise_protocol::Basis,
awaited: bool,
}
fn sent(
client: &mut ClientState,
host: &mut App,
event: KeyEvent,
window: (usize, usize),
) -> Unanswered {
let KeyOutcome::Local(cmds) = resolve(client, event) else {
panic!("{event:?}: the client must answer this key locally");
};
client.fold_outgoing(&cmds);
apply_on(host, &cmds);
let awaited = cmds
.iter()
.any(|c| matches!(c, improvise_protocol::Command::Model { .. }));
let (grid, basis) = (client_of(host, window).grid, published_basis(host, window));
Unanswered {
grid,
basis,
awaited,
}
}
fn answered(client: &mut ClientState, answer: Unanswered) {
client.grid = answer.grid;
improvise_protocol::WidgetCache::apply(&mut client.basis, &answer.basis);
if answer.awaited {
client.accepted();
}
}
/// BUG (improvise-mdfc) — see `context/test-evidence/clients.md`.
#[test]
fn a_tab_sent_before_the_last_write_is_answered_reopens_on_that_write() {
let window = (2, 2);
let mut client_host = precise_host(improvise_protocol::CellValue::Number(4.0));
client_host.cursor_mut().selected = (0, 1);
let mut key_host = precise_host(improvise_protocol::CellValue::Number(4.0));
key_host.cursor_mut().selected = (0, 1);
let mut client = client_ready_to_edit(&client_host, window);
let tab = BrowserKey::new("Tab").to_key_event().unwrap();
for key in ["i", "9"] {
let event = BrowserKey::new(key).to_key_event().unwrap();
sent_and_answered(&mut client, &mut client_host, event, window);
reduce_key(&mut key_host, event);
}
// Three Tabs, so a frame that writes nothing sits between the write and the last.
let answers: Vec<Unanswered> = (0..3)
.map(|_| sent(&mut client, &mut client_host, tab, window))
.collect();
let awaited: Vec<bool> = answers.iter().map(|a| a.awaited).collect();
assert_eq!(
awaited,
[true, false, false],
"setup: only the first Tab writes"
);
for answer in answers {
reduce_key(&mut key_host, tab);
answered(&mut client, answer);
}
assert_eq!(
client.view.buffers, key_host.view_state.buffers,
"the second editor opened on the cache the first write had made stale"
);
for key in ["5", "Enter"] {
let event = BrowserKey::new(key).to_key_event().unwrap();
sent_and_answered(&mut client, &mut client_host, event, window);
reduce_key(&mut key_host, event);
}
for (end, app) in [("browser", &client_host), ("host", &key_host)] {
assert_eq!(
app.model_state
.workbook
.model
.get_cell(&region_measure("North", "Revenue")),
Some(&improvise_protocol::CellValue::Number(295.0)),
"{end}: the browser overwrote the value Tab had just written"
);
}
}
/// BUG (improvise-p6v6) — see `context/test-evidence/clients.md`.
#[test]
fn an_editor_opened_with_i_before_the_last_write_is_answered_opens_on_that_write() {
let window = (2, 2);
let mut client_host = precise_host(improvise_protocol::CellValue::Number(4.0));
client_host.cursor_mut().selected = (0, 1);
let mut key_host = precise_host(improvise_protocol::CellValue::Number(4.0));
key_host.cursor_mut().selected = (0, 1);
let mut client = client_ready_to_edit(&client_host, window);
for key in ["i", "9"] {
let event = BrowserKey::new(key).to_key_event().unwrap();
sent_and_answered(&mut client, &mut client_host, event, window);
reduce_key(&mut key_host, event);
}
// Tab writes 29; Esc and i reopen before its answer comes back.
let answers: Vec<Unanswered> = ["Tab", "Escape", "i"]
.into_iter()
.map(|key| {
let event = BrowserKey::new(key).to_key_event().unwrap();
reduce_key(&mut key_host, event);
sent(&mut client, &mut client_host, event, window)
})
.collect();
let awaited: Vec<bool> = answers.iter().map(|a| a.awaited).collect();
assert_eq!(awaited, [true, false, false], "setup: only the Tab writes");
answers
.into_iter()
.for_each(|answer| answered(&mut client, answer));
assert_eq!(
client.view.buffers, key_host.view_state.buffers,
"i opened on the cache the unanswered write had made stale"
);
for key in ["5", "Enter"] {
let event = BrowserKey::new(key).to_key_event().unwrap();
sent_and_answered(&mut client, &mut client_host, event, window);
reduce_key(&mut key_host, event);
}
for (end, app) in [("browser", &client_host), ("host", &key_host)] {
assert_eq!(
app.model_state
.workbook
.model
.get_cell(&region_measure("North", "Revenue")),
Some(&improvise_protocol::CellValue::Number(295.0)),
"{end}: the browser overwrote the value Tab had just written"
);
}
}
const FORMULAS: [&str; 3] = [
"Profit = Revenue * 2",
"Tax = Revenue * 0.1",
flake.lock
Generated
+16
View File
@@ -45,6 +45,21 @@
"type": "github"
}
},
"crane": {
"locked": {
"lastModified": 1789760033,
"narHash": "sha256-jtT4yxZpR8seYnlCMMWSSPlFN92zO6ICuZ8pJrmi86k=",
"owner": "ipetkov",
"repo": "crane",
"rev": "73b980519cefc727a5f6cc8e5c0947a2f9be6edd",
"type": "github"
},
"original": {
"owner": "ipetkov",
"repo": "crane",
"type": "github"
}
},
"crate2nix": {
"inputs": {
"cachix": "cachix",
@@ -341,6 +356,7 @@
"root": {
"inputs": {
"beads": "beads",
"crane": "crane",
"crate2nix": "crate2nix",
"flake-utils": "flake-utils",
"nixpkgs": "nixpkgs_4",
flake.nix
+84 -22
View File
@@ -9,6 +9,7 @@
};
flake-utils.url = "github:numtide/flake-utils";
crate2nix.url = "github:nix-community/crate2nix";
crane.url = "github:ipetkov/crane";
};
outputs = {
@@ -18,6 +19,7 @@
rust-overlay,
flake-utils,
crate2nix,
crane,
}:
flake-utils.lib.eachDefaultSystem (system: let
pkgs = import nixpkgs {
@@ -36,12 +38,75 @@
cargoNix = import ./Cargo.nix {
inherit nixpkgs pkgs;
};
# wasm-pack downloads wasm-bindgen and binaryen from GitHub releases
# unless they are on PATH, and its downloader ignores SSL_CERT_FILE, so
# behind the runners' TLS-auditing proxy those downloads fail (binaryen
# with no fallback). On PATH, it downloads nothing. wasm-pack only takes
# a PATH wasm-bindgen whose version equals Cargo.lock's `wasm-bindgen`,
# so this pin moves with the lock: on a mismatch the sandboxed build fails.
wasmBindgenCli = pkgs.buildWasmBindgenCli rec {
src = pkgs.fetchCrate {
pname = "wasm-bindgen-cli";
version = "0.2.128";
hash = "sha256-a7lcXJnnZkYReja+iUO7NqqrWyv3toxnUgQb8s4IS5s=";
};
cargoDeps = pkgs.rustPlatform.fetchCargoVendor {
inherit src;
inherit (src) pname version;
hash = "sha256-R1Tas33Ursy8kqsxguAkG0ZhNed2n5uFTAhw1l2qlLY=";
};
};
# Everything CI runs, and nothing else: `devShells.ci` is exactly this,
# and the default shell is this plus editor and release tooling, so a
# local `nix develop` runs the toolchain the checks run.
ciInputs = [
rustToolchain
pkgs.pkg-config
# `scripts/kb` and the CI knowledge-base check both run trealla. It
# was reached through `nix shell nixpkgs#trealla` per invocation,
# which cost more than the work: the tool's first real use took past
# two minutes for a write that takes half a second on PATH.
pkgs.trealla
# `web/build.sh` — and so the Pages job's local dry run — needs this.
# The `wasm` CI job only runs `cargo check`, which does not, so the
# shell was missing the one tool that turns a passing check into a
# loadable page (improvise-d31). CI takes whatever version this is.
pkgs.wasm-pack
wasmBindgenCli
pkgs.binaryen
# The js and browser jobs once ran 22 while `build` ran 24; one shell
# means one node.
pkgs.nodejs_24
pkgs.python3
pkgs.jq
];
# Playwright's downloaded browsers are generic-Linux builds that cannot
# start on NixOS, so nixpkgs' build is used. harness/package.json pins
# @playwright/test to this playwright-driver's version: they must match.
craneLib = (crane.mkLib pkgs).overrideToolchain (_: rustToolchain);
ci = import ./nix/ci.nix {
inherit pkgs craneLib wasmBindgenCli;
src = self;
# What the site's cache-bust token names; CI checks out a clean tree.
rev = self.rev or "dirty";
};
playwrightEnv = {
PLAYWRIGHT_BROWSERS_PATH = "${pkgs.playwright-driver.browsers}";
PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD = "1";
PLAYWRIGHT_SKIP_VALIDATE_HOST_REQUIREMENTS = "true";
RUST_BACKTRACE = "1";
};
in {
inherit cargoNix;
devShells.default = pkgs.mkShell {
nativeBuildInputs = [
rustToolchain
pkgs.pkg-config
devShells.ci = pkgs.mkShell (playwrightEnv // {nativeBuildInputs = ciInputs;});
devShells.default = pkgs.mkShell (playwrightEnv // {
nativeBuildInputs = ciInputs ++ [
pkgs.rust-analyzer
crate2nix.packages.${system}.default
pkgs.cargo-expand
@@ -50,18 +115,6 @@
pkgs.universal-ctags
beads.packages.${system}.default
# `scripts/kb` and the CI knowledge-base check both run trealla. It
# was reached through `nix shell nixpkgs#trealla` per invocation,
# which cost more than the work: the tool's first real use took past
# two minutes for a write that takes half a second on PATH.
pkgs.trealla
# `web/build.sh` — and so the Pages job's local dry run — needs this.
# The `wasm` CI job only runs `cargo check`, which does not, so the
# shell was missing the one tool that turns a passing check into a
# loadable page (improvise-d31).
pkgs.wasm-pack
# Demo recording and release tooling
pkgs.asciinema
pkgs.vhs
@@ -69,12 +122,21 @@
# nixpkgs cargo-dist installs as "dist"; alias so `cargo dist` works
(pkgs.writeShellScriptBin "cargo-dist" ''exec ${pkgs.cargo-dist}/bin/dist "$@"'')
];
RUST_BACKTRACE = "1";
};
});
packages = {
improvise = cargoNix.workspaceMembers.improvise.build;
default = self.packages.${system}.improvise;
};
packages =
ci.packages
// {
improvise = cargoNix.workspaceMembers.improvise.build;
default = self.packages.${system}.improvise;
};
# `nix flake check` builds every CI build: crane's checks plus the
# crate2nix package and the bundles.
checks =
ci.checks
// {
inherit (self.packages.${system}) improvise web;
};
});
}
harness/package-lock.json
Generated
+15 -33
View File
@@ -6,70 +6,52 @@
"": {
"name": "improvise-harness",
"devDependencies": {
"@playwright/test": "1.56.1"
"@playwright/test": "1.63.0"
}
},
"node_modules/@playwright/test": {
"version": "1.56.1",
"resolved": "https://registry.npmjs.org/@playwright/test/-/test-1.56.1.tgz",
"integrity": "sha512-vSMYtL/zOcFpvJCW71Q/OEGQb7KYBPAdKh35WNSkaZA75JlAO8ED8UN6GUNTm3drWomcbcqRPFqQbLae8yBTdg==",
"version": "1.63.0",
"resolved": "https://registry.npmjs.org/@playwright/test/-/test-1.63.0.tgz",
"integrity": "sha512-oxMK4vllB9RK5NQ2l1pq1IfOf2AvnEuj/vYGDj0H2nMtmtZpKtCwt/l00GEO6xjGfpBNAvjovvYdCm50dRQkpQ==",
"dev": true,
"license": "Apache-2.0",
"dependencies": {
"playwright": "1.56.1"
"playwright": "1.63.0"
},
"bin": {
"playwright": "cli.js"
},
"engines": {
"node": ">=18"
}
},
"node_modules/fsevents": {
"version": "2.3.2",
"resolved": "https://registry.npmjs.org/fsevents/-/fsevents-2.3.2.tgz",
"integrity": "sha512-xiqMQR4xAeHTuB9uWm+fFRcIOgKBMiOBP+eXiyT7jsgVCq1bkVygt00oASowB7EdtpOHaaPgKt812P9ab+DDKA==",
"dev": true,
"hasInstallScript": true,
"license": "MIT",
"optional": true,
"os": [
"darwin"
],
"engines": {
"node": "^8.16.0 || ^10.6.0 || >=11.0.0"
"node": ">=20"
}
},
"node_modules/playwright": {
"version": "1.56.1",
"resolved": "https://registry.npmjs.org/playwright/-/playwright-1.56.1.tgz",
"integrity": "sha512-aFi5B0WovBHTEvpM3DzXTUaeN6eN0qWnTkKx4NQaH4Wvcmc153PdaY2UBdSYKaGYw+UyWXSVyxDUg5DoPEttjw==",
"version": "1.63.0",
"resolved": "https://registry.npmjs.org/playwright/-/playwright-1.63.0.tgz",
"integrity": "sha512-+7ziBLidS4NaNCdt57SUDT+wYmmd5fmiQejUic/kb+YsYSCPyOOE9sebzMjNmQrsnNpDJqd4WHvV/8lfKfUDUg==",
"dev": true,
"license": "Apache-2.0",
"dependencies": {
"playwright-core": "1.56.1"
"playwright-core": "1.63.0"
},
"bin": {
"playwright": "cli.js"
},
"engines": {
"node": ">=18"
},
"optionalDependencies": {
"fsevents": "2.3.2"
"node": ">=20"
}
},
"node_modules/playwright-core": {
"version": "1.56.1",
"resolved": "https://registry.npmjs.org/playwright-core/-/playwright-core-1.56.1.tgz",
"integrity": "sha512-hutraynyn31F+Bifme+Ps9Vq59hKuUCz7H1kDOcBs+2oGguKkWTU50bBWrtz34OUWmIwpBTWDxaRPXrIXkgvmQ==",
"version": "1.63.0",
"resolved": "https://registry.npmjs.org/playwright-core/-/playwright-core-1.63.0.tgz",
"integrity": "sha512-rYCsBF/M5HjUch52bbtVONEFjv6Xu8sm8h72dNlR5bzIE1fvC/bxgspzkjSfU+MweEMmPM8KJebG6nnyxo5mCg==",
"dev": true,
"license": "Apache-2.0",
"bin": {
"playwright-core": "cli.js"
},
"engines": {
"node": ">=18"
"node": ">=20"
}
}
}
harness/package.json
+1 -1
View File
@@ -8,6 +8,6 @@
"test": "playwright test"
},
"devDependencies": {
"@playwright/test": "1.56.1"
"@playwright/test": "1.63.0"
}
}
nix/ci.nix
+184
View File
@@ -0,0 +1,184 @@
# Everything CI *builds*, as Nix derivations (crane). The script-only checks
# (node tests, kb, the checkers, the comment and CI-graph gates, fmt) stay dev
# shell steps in ci.yml: they compile nothing, and their `#!/usr/bin/env bash`
# shebangs -- which scripts/checkers.sh itself enforces -- cannot run in the
# build sandbox, which has no /usr/bin/env.
#
# Why Nix: the self-hosted runners substitute from titan's harmonia first, and
# titan builds gh/main's outputs hourly. Dependencies are separate derivations
# (buildDepsOnly, keyed on the Cargo.lock), so a PR that leaves the lock alone
# rebuilds only the workspace.
{
pkgs,
craneLib,
wasmBindgenCli,
src,
rev,
}: let
inherit (pkgs) lib;
# Two lockfiles: harness/opfs-contract is its own workspace root.
cargoVendorDir = craneLib.vendorMultipleCargoDeps {
cargoLockList = [../Cargo.lock ../harness/opfs-contract/Cargo.lock];
};
common = {
inherit src cargoVendorDir;
pname = "improvise";
version = "0";
strictDeps = true;
# The `test` job ran the dev/test profiles, not crane's default release:
# debug assertions and overflow checks are part of what it tested.
CARGO_PROFILE = "";
};
wasmTarget = "wasm32-unknown-unknown";
# The crates in the browser bundle, checked for wasm32 one at a time: a
# single `cargo check -p a -p b` unifies features across them and can hide
# a crate that only builds with a sibling's features (improvise-ywd).
wasmCrates = [
"improvise-core"
"improvise-formula"
"improvise-protocol"
"improvise-io"
"improvise-command"
"improvise-wasm-client"
"improvise-worker-server"
"improvise-gp"
];
pFlags = crates: lib.concatMapStringsSep " " (c: "-p ${c}") crates;
nativeDeps = craneLib.buildDepsOnly (common
// {
pname = "improvise-deps";
cargoExtraArgs = "--locked --workspace";
});
wasmCheckDeps = craneLib.buildDepsOnly (common
// {
pname = "improvise-wasm-check-deps";
cargoExtraArgs = "--locked --target ${wasmTarget} ${pFlags wasmCrates}";
cargoCheckExtraArgs = "";
doCheck = false;
});
# What wasm-pack's `cargo build --release --target wasm32` needs.
bundleDeps = craneLib.buildDepsOnly (common
// {
pname = "improvise-bundle-deps";
CARGO_PROFILE = "release";
cargoExtraArgs = "--locked --target ${wasmTarget} ${pFlags ["improvise-wasm-client" "improvise-worker-server"]}";
cargoCheckCommand = "true";
doCheck = false;
});
# A derivation that runs cargo commands and only has to succeed.
cargoCheck = name: args:
craneLib.mkCargoDerivation (common
// {
pname = "improvise-${name}";
doInstallCargoArtifacts = false;
installPhaseCommand = "mkdir -p $out";
}
// args);
in rec {
checks = {
clippy = craneLib.cargoClippy (common
// {
pname = "improvise-clippy";
cargoArtifacts = nativeDeps;
cargoClippyExtraArgs = "--workspace --all-targets -- -D warnings";
});
# The evidence gate runs `cargo test --workspace -- --list`, so it runs
# here, where the tests are already compiled, not as a separate step.
test = craneLib.cargoTest (common
// {
pname = "improvise-test";
cargoArtifacts = nativeDeps;
cargoTestExtraArgs = "--workspace";
nativeBuildInputs = [pkgs.python3 pkgs.git];
# tests/shipped_documents_load.rs walks the git index (`git ls-files`)
# and makes scratch repos. The flake source is exactly the tracked
# files, so force-adding all of it (but crane's target/) rebuilds the
# checkout's index, ignored-but-tracked documents included.
preBuild = ''
export HOME=$TMPDIR
export GIT_AUTHOR_NAME=nix GIT_AUTHOR_EMAIL=nix@localhost
export GIT_COMMITTER_NAME=nix GIT_COMMITTER_EMAIL=nix@localhost
git init -q .
git add --all --force -- . ':!target'
'';
postBuild = ''
python3 scripts/check-comment-runs.py --check-evidence
'';
});
wasm-check = cargoCheck "wasm-check" {
cargoArtifacts = wasmCheckDeps;
buildPhaseCargoCommand = lib.concatMapStringsSep "\n" (c: ''
echo "== ${c}"
cargo check --locked -p ${c} --target ${wasmTarget}
'')
wasmCrates;
};
# harness/opfs-contract is its own workspace, so the root clippy never
# reaches it; `-p` keeps cargo fmt to this one crate (see ci.yml).
harness-lint = cargoCheck "harness-lint" {
cargoArtifacts = null;
buildPhaseCargoCommand = ''
cd harness/opfs-contract
cargo fmt -p opfs-contract -- --check
cargo clippy --locked --target ${wasmTarget} --all-targets -- -D warnings
'';
};
};
packages = {
# The dependency builds on their own, for titan's cache timer to keep as a
# GC root: the checks' outputs do not reference them, so rooting only the
# checks would let the part a PR actually reuses be collected.
ci-deps = pkgs.linkFarmFromDrvs "improvise-ci-deps" [nativeDeps wasmCheckDeps bundleDeps];
# harness/build.sh, unchanged: the three wasm bundles and harness/serve.
# No network: wasm-bindgen and wasm-opt come from PATH, so wasm-pack
# downloads nothing (see wasmBindgenCli in flake.nix).
web = craneLib.mkCargoDerivation (common
// {
pname = "improvise-web";
cargoArtifacts = bundleDeps;
doInstallCargoArtifacts = false;
nativeBuildInputs = [pkgs.wasm-pack wasmBindgenCli pkgs.binaryen pkgs.nodejs_24];
# web/assemble.sh calls scripts/assemble-stamp.sh, whose
# `#!/usr/bin/env bash` has no interpreter in the sandbox. Patching
# this private copy is harmless: no checker runs here.
postPatch = "patchShebangs scripts web harness";
buildPhaseCargoCommand = ''
export HOME=$TMPDIR
./harness/build.sh
'';
installPhaseCommand = ''
mkdir -p $out
cp -r harness/serve $out/harness-serve
cp -r web/pkg web/worker-pkg $out/
'';
});
# The Pages tree, stamped with the commit it was built from. Separate from
# `web` so the stamp changing every commit does not rebuild the bundles.
site =
pkgs.runCommand "improvise-site" {
nativeBuildInputs = [pkgs.nodejs_24];
} ''
cp -r ${src} tree
chmod -R u+w tree
patchShebangs tree/scripts tree/web
cp -r ${packages.web}/pkg ${packages.web}/worker-pkg tree/web/
chmod -R u+w tree/web
cd tree
./web/assemble.sh $out ${rev}
'';
};
}
scripts/bd-audit-ids
+38 -8
View File
@@ -12,7 +12,9 @@ Run from anywhere inside the repository. Takes no arguments.
exit 0 every cited id is a ticket bd lists
exit 1 at least one cited id is not; each is printed
exit 3 bd could not answer: missing, failing, or listing nothing
exit 4 usage error, or the file scan itself failed
exit 4 usage error, not inside a git repository, or a cited file
could not be read or is absent (only a file `git ls-files -d`
reports as deleted is skipped)
The comparison is POSITIVE: a cited id passes only by appearing in what
`bd list --all` returns. The recipe this replaced looked for bd's "not found"
@@ -36,15 +38,41 @@ EXCLUDE='^improvise-(core|formula|io|protocol|command|gp|ws|worker|wasm|harness|
work=$(mktemp -d)
trap 'rm -rf "$work"' EXIT
cd "$(git rev-parse --show-toplevel)"
git ls-files -z --cached --others --exclude-standard -- "${ROOTS[@]}" > "$work/files"
top=$(git rev-parse --show-toplevel 2>/dev/null) || {
echo "bd-audit-ids: not inside a git repository, so nothing was checked." >&2
exit 4
}
cd "$top"
# A deleted tracked file is skipped ON PURPOSE: its ids are leaving the tree.
# Only `ls-files -d` says deleted; any other absent path is exit 4 (lqs0, tdfd).
: > "$work/absent"
git ls-files -z --cached --others --exclude-standard -- "${ROOTS[@]}" |
while IFS= read -r -d '' f; do
if [ -e "$f" ] || [ -L "$f" ]; then printf '%s\0' "$f"
elif [ -z "$(git --literal-pathspecs ls-files -d -- "$f")" ]; then
printf '%s\n' "$f" >> "$work/absent"
fi
done > "$work/files"
if [ -s "$work/absent" ]; then
echo "bd-audit-ids: tracked but absent, and not deleted, so never read:" >&2
sed 's/^/ /' "$work/absent" >&2
exit 4
fi
# xargs exits 123 when one grep batch matched nothing, which is not an error.
# xargs reports grep's no-match (1) and its read error (2) alike, as 123, so
# each batch maps 1 to success and 2 to 255: xargs stops, nonzero (GNU 124, BSD 1).
: > "$work/raw"
set +e
xargs -0 grep -d skip -ohE 'improvise-[a-z0-9]{3,8}\b' < "$work/files" > "$work/raw"
if [ -s "$work/files" ]; then
xargs -0 bash -c 'grep -d skip -ohE "$0" "$@"; [ $? -le 1 ] || exit 255' \
'improvise-[a-z0-9]{3,8}\b' < "$work/files" > "$work/raw"
fi
rc=$?
set -e
[ "$rc" -eq 0 ] || [ "$rc" -eq 123 ] || { echo "bd-audit-ids: grep failed ($rc)" >&2; exit 4; }
[ "$rc" -eq 0 ] || {
echo "bd-audit-ids: a cited file could not be read (xargs $rc), so the audit is incomplete." >&2
exit 4
}
sort -u "$work/raw" | { grep -vE "$EXCLUDE" || true; } > "$work/cited"
if ! command -v "$BD" >/dev/null 2>&1; then
@@ -56,10 +84,12 @@ if ! "$BD" list --all --limit 0 --json > "$work/known.json" 2> "$work/bd.err"; t
cat "$work/bd.err" >&2
exit 3
fi
if ! jq -r '.[].id' "$work/known.json" 2>/dev/null | sort -u > "$work/known"; then
echo "bd-audit-ids: '$BD list' did not print a JSON list of issues." >&2
if ! jq -e 'type == "array" and all(.[]; (.id | type) == "string")' \
"$work/known.json" > /dev/null 2>&1; then
echo "bd-audit-ids: '$BD list' did not print a JSON list of issues with ids." >&2
exit 3
fi
jq -r '.[].id' "$work/known.json" | sort -u > "$work/known"
if [ ! -s "$work/known" ]; then
echo "bd-audit-ids: '$BD list' returned no issues; refusing to call that a pass." >&2
exit 3
scripts/bd-audit-ids-test.sh
+66
View File
@@ -77,6 +77,72 @@ got=$?
[ "$got" = 3 ] && ok "the real tree does not pass against a silent bd" \
|| bad "the real tree does not pass against a silent bd" "exit $got"
# improvise-lqs0: a file the scan cannot read, and the two exit-code mismatches.
# A second repo, so the file list is exactly these cases' files.
r2="$work/repo2"
mkdir -p "$r2/context"
git -C "$r2" init -q
printf 'improvise-ab12\n' > "$r2/context/a.md"
printf 'improvise-dead9\n' > "$r2/context/b.md"
git -C "$r2" add -A
git -C "$r2" -c user.name=t -c user.email=t@t commit -qm fixture
export FAKE_BD_OUT="$work/known.json" FAKE_BD_RC=0
(cd "$r2" && "$AUDIT") > "$work/out" 2>&1
got=$?
[ "$got" = 1 ] && grep -q 'improvise-dead9' "$work/out" \
&& ok "control: both files readable, the dead id is found" \
|| bad "control: both files readable, the dead id is found" "exit $got"
# A symlink whose target is gone reads as an error for any user, root included.
ln -s missing-target "$r2/context/c.md"
(cd "$r2" && "$AUDIT") > "$work/out" 2>&1
got=$?
[ "$got" = 4 ] && ok "a cited file grep cannot read is exit 4, not a pass" \
|| bad "a cited file grep cannot read is exit 4, not a pass" "exit $got: $(head -c 200 "$work/out")"
rm -f "$r2/context/c.md"
if [ "$(id -u)" != 0 ]; then
chmod 000 "$r2/context/b.md"
(cd "$r2" && "$AUDIT") > "$work/out" 2>&1
got=$?
chmod 644 "$r2/context/b.md"
[ "$got" = 4 ] && ok "an unreadable cited file is exit 4, not a pass" \
|| bad "an unreadable cited file is exit 4, not a pass" "exit $got: $(head -c 200 "$work/out")"
fi
# Decided, not accidental: a tracked file deleted from the working tree is not
# scanned. Its ids are leaving the tree, so its dead id is not reported.
mv "$r2/context/b.md" "$work/b.md.away"
(cd "$r2" && "$AUDIT") > "$work/out" 2>&1
got=$?
mv "$work/b.md.away" "$r2/context/b.md"
[ "$got" = 0 ] && ok "a tracked file deleted from the working tree is not scanned" \
|| bad "a tracked file deleted from the working tree is not scanned" "exit $got: $(head -c 200 "$work/out")"
# Absent is not deleted: a skip-worktree path is missing too, but ls-files -d
# does not list it, so its ids were never read (improvise-tdfd).
git -C "$r2" update-index --skip-worktree context/b.md
mv "$r2/context/b.md" "$work/b.md.away"
(cd "$r2" && "$AUDIT") > "$work/out" 2>&1
got=$?
mv "$work/b.md.away" "$r2/context/b.md"
git -C "$r2" update-index --no-skip-worktree context/b.md
[ "$got" = 4 ] && grep -q 'context/b.md' "$work/out" \
&& ok "a skip-worktree file missing from the tree is exit 4, and named" \
|| bad "a skip-worktree file missing from the tree is exit 4, and named" "exit $got: $(head -c 200 "$work/out")"
mkdir -p "$work/not-a-repo"
(cd "$work/not-a-repo" && GIT_CEILING_DIRECTORIES="$work" "$AUDIT") > "$work/out" 2>&1
got=$?
[ "$got" = 4 ] && ok "run outside a git repository is exit 4" \
|| bad "run outside a git repository is exit 4" "exit $got"
printf '[{"title":"no id here"}]\n' > "$work/noid.json"
FAKE_BD_OUT="$work/noid.json" \
run_case "a bd listing issues with no id is exit 3, not every id dead" 3 'did not print'
unset FAKE_BD_OUT FAKE_BD_RC
"$AUDIT" --help > "$work/out" 2>&1
got=$?
[ "$got" = 4 ] && grep -q 'POSITIVE' "$work/out" \
scripts/checkers-test.sh
+4 -43
View File
@@ -67,21 +67,7 @@ direct=$?
unset name
# ---------------------------------------------------------------------------
# 2. The bash-5 `local` semantics. A multi-assignment `local` does not read
# left to right on bash 5, so this suite passes on macOS bash 3.2 and dies
# under `set -u` on bash 5 -- PR #133's 19/19 local against 2/19 in CI.
d="$(sandbox bash5)"
suite "$d" vers <<'EOF'
#!/usr/bin/env bash
set -euo pipefail
f() { local a="$1" b="$a/x"; echo "$b"; }
f one
EOF
expect "a bash-3.2-only construct fails under the runner's pinned bash" \
1 "$d/checkers.sh" vers
# ---------------------------------------------------------------------------
# 3. The shebang rule. `#!/bin/sh` is dash in CI and bash-in-sh-mode on macOS.
# 2. The shebang rule. `#!/bin/sh` is dash in CI and bash-in-sh-mode on macOS.
d="$(sandbox shebang)"
suite "$d" fine <<'EOF'
#!/usr/bin/env bash
@@ -101,7 +87,7 @@ rm -f "$d/legacy.sh"
expect "and the same sandbox passes once it is removed" 0 "$d/checkers.sh" fine
# ---------------------------------------------------------------------------
# 4. A failing suite is reported, and a passing one is not.
# 3. A failing suite is reported, and a passing one is not.
d="$(sandbox rc)"
suite "$d" red <<'EOF'
#!/usr/bin/env bash
@@ -117,32 +103,7 @@ EOF
expect "a passing suite passes the runner" 0 "$d/checkers.sh" blue
# ---------------------------------------------------------------------------
# 5. No bash >= 5 and no nix is exit 3, not a silent run under whatever is
# there. This is the arm that would have refused the macOS system bash.
d="$(sandbox oldbash)"
suite "$d" any <<'EOF'
#!/usr/bin/env bash
exit 0
EOF
mkdir -p "$work/fakebin"
cat > "$work/fakebin/bash" <<'EOF'
#!/bin/sh
case "$*" in *BASH_VERSINFO*) echo 3; exit 0 ;; esac
exit 0
EOF
chmod +x "$work/fakebin/bash"
# The runner is invoked through a REAL bash, so the fake one below is only
# what its own `command -v bash` probe finds -- not its interpreter.
# /usr/bin:/bin carries the utilities the runner itself needs and carries no
# nix, so the fake bash is the only candidate and the nix fallback is absent.
env PATH="$work/fakebin:/usr/bin:/bin" HOME="$HOME" \
"${BASH:-bash}" "$d/checkers.sh" any > "$work/out" 2>&1
got=$?
[ "$got" = 3 ] && ok "an old bash with no nix is exit 3" \
|| bad "an old bash with no nix is exit 3" "exit $got, wanted 3"
# ---------------------------------------------------------------------------
# 6. A named suite that does not exist is a usage error, not a silent no-op.
# 4. A named suite that does not exist is a usage error, not a silent no-op.
d="$(sandbox missing)"
suite "$d" present <<'EOF'
#!/usr/bin/env bash
@@ -151,7 +112,7 @@ EOF
expect "an unknown suite name is exit 4" 4 "$d/checkers.sh" absent
# ---------------------------------------------------------------------------
# 7. --list names every suite in the directory and nothing else.
# 5. --list names every suite in the directory and nothing else.
d="$(sandbox listing)"
suite "$d" alpha <<'EOF'
#!/usr/bin/env bash
scripts/checkers.sh
+10 -36
View File
@@ -3,15 +3,14 @@
# and so does the local gate, so a suite cannot be green under one shell and
# red under the other. Reasoning: context/code/tooling.md section 3.
#
# It does three things a bare `./scripts/X-test.sh` does not:
# It does two things a bare `./scripts/X-test.sh` does not. It runs every
# suite under the bash running it (the dev shell's), and it:
#
# 1. pins a bash >= 5, taken from nix when the ambient one is older, so the
# macOS system bash 3.2 can never be the shell a suite passes under;
# 2. scrubs the environment down to an allowlist, because `nix develop`
# 1. scrubs the environment down to an allowlist, because `nix develop`
# exports `name`, `out`, `system`, `shell`, `builder` and two dozen more
# generically-named variables that CI does not have -- a `set -u` fault
# reading one of them is SILENT inside the dev shell and fatal in CI;
# 3. refuses a shell script in scripts/ whose shebang is not
# 2. refuses a shell script in scripts/ whose shebang is not
# `#!/usr/bin/env bash`, so the sh/bash split cannot come back.
set -euo pipefail
@@ -30,7 +29,6 @@ usage: checkers.sh [--list] [--self-test] [name ...]
exit 0 every suite passed
exit 1 a suite failed, or a shebang is not `#!/usr/bin/env bash`
exit 3 no bash >= 5 could be found and nix could not supply one
exit 4 usage error
THIS SCRIPT'S `$?` MUST BE THE ONE YOU READ. Nothing may follow it in the same
@@ -46,19 +44,6 @@ ALLOW="HOME TMPDIR TMP TEMP TERM LANG LC_ALL USER LOGNAME
NIX_PATH NIX_REMOTE NIX_SSL_CERT_FILE SSL_CERT_FILE
XDG_CACHE_HOME XDG_CONFIG_HOME XDG_DATA_HOME"
# Prints the absolute path of a bash >= 5, or nothing. The ambient one is tried
# first so a machine that already has a modern bash pays no nix startup at all.
find_modern_bash() {
local candidate major
candidate="$(command -v bash || true)"
if [ -n "$candidate" ]; then
major="$("$candidate" -c 'echo "${BASH_VERSINFO[0]}"' 2>/dev/null || echo 0)"
if [ "$major" -ge 5 ] 2>/dev/null; then echo "$candidate"; return 0; fi
fi
command -v nix >/dev/null 2>&1 || return 0
nix shell nixpkgs#bash --command bash -c 'command -v bash' 2>/dev/null || true
}
# Every shell script in scripts/ says `#!/usr/bin/env bash` and nothing else.
# `#!/bin/sh` is dash in CI and bash 3.2 in sh-mode on macOS: a second platform
# split, undetected for the same reason the first one was.
@@ -91,19 +76,16 @@ suites() {
done
}
# $1 the pinned bash, $2 the suite file. The suite runs with the pinned bash
# FIRST on PATH, so a script it invokes through its own `#!/usr/bin/env bash`
# -- or through `sh` -- gets the pinned one too, not the system 3.2.
# $1 the suite file, run by the bash running this script, with PATH unchanged.
run_suite() {
local pinned="$1" file="$2"
local bindir; bindir="$(dirname "$pinned")"
local file="$1"
local -a env_args=(-i)
local v
for v in $ALLOW ${CHECKERS_KEEP:-}; do
if [ -n "${!v+set}" ]; then env_args+=("$v=${!v}"); fi
done
env_args+=("PATH=$bindir:${PATH:-/usr/bin:/bin}")
env "${env_args[@]}" "$pinned" "$file"
env_args+=("PATH=$PATH")
env "${env_args[@]}" "$BASH" "$file"
}
main() {
@@ -120,15 +102,7 @@ main() {
shift
done
local pinned; pinned="$(find_modern_bash)"
if [ -z "$pinned" ]; then
echo 'checkers: no bash >= 5 on PATH and nix cannot supply one.' >&2
echo ' Enter the dev shell (direnv allow / nix develop) and retry.' >&2
return 3
fi
printf 'checkers: bash %s at %s\n' \
"$("$pinned" -c 'echo "${BASH_VERSINFO[0]}.${BASH_VERSINFO[1]}"')" "$pinned"
printf 'checkers: bash %s at %s\n' "$BASH_VERSION" "$BASH"
local rc=0
check_shebangs || rc=1
@@ -146,7 +120,7 @@ main() {
rc=4; continue
fi
printf '\n=== %s ===\n' "$n"
run_suite "$pinned" "$file" || rc=1
run_suite "$file" || rc=1
done
return $rc
}
scripts/kb
+1 -1
View File
@@ -87,7 +87,7 @@ ledger_path() {
# first line; bytes alone are not enough, since a restore can reproduce them.
file_stamp() {
printf '%s %s' "$(stamp_of "$1")" \
"$(stat -c '%i %.9Y' "$1" 2>/dev/null || stat -f '%i %Fm' "$1")"
"$(stat -c '%i %.9Y' "$1")"
}
ledger_holds() { [ -f "$1" ] && [ "$(head -n 1 "$1")" = "% kb-stamp $(file_stamp "$2")" ]; }
scripts/pr-reviews-test.sh
+24 -7
View File
@@ -5,10 +5,26 @@
set -uo pipefail
here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
REVIEWS="$here/pr-reviews"
work="$(mktemp -d)"
trap 'rm -rf "$work"' EXIT
# Every run is bounded, so a loop that never stops FAILS (exit 124) rather than
# hanging the suite. The real sleep, resolved before any case stubs PATH; NixOS has no /bin/sleep.
BOUND=20
REAL_SLEEP="$(type -P sleep)"
bounded_reviews() {
"$here/pr-reviews" "$@" &
local pid=$! dog done rc
"$REAL_SLEEP" "$BOUND" &
dog=$!
wait -n -p done "$pid" "$dog"; rc=$?
if [ "$done" = "$pid" ]; then kill "$dog" 2>/dev/null; wait "$dog" 2>/dev/null; return "$rc"; fi
kill -TERM "$pid" 2>/dev/null; wait "$pid" 2>/dev/null
echo "pr-reviews-test: killed after ${BOUND}s; the --wait loop never stopped" >&2
return 124
}
REVIEWS=bounded_reviews
pass=0; fail=0
ok() { printf ' ok %s\n' "$1"; pass=$((pass+1)); }
bad() { printf ' FAIL %s (%s)\n' "$1" "$2"; fail=$((fail+1)); }
@@ -203,7 +219,7 @@ clock="$work/clock"
mkdir -p "$clock"
cat > "$clock/date" <<'EOF'
#!/usr/bin/env bash
[ "${1:-}" = "+%s" ] || exec /bin/date "$@"
[ "$*" = "+%s" ] || { echo "fake date: only +%s is faked, got: $*" >&2; exit 2; }
n=$(( $(cat "$CLOCK_TICKS" 2>/dev/null || echo 0) + 1 ))
echo "$n" > "$CLOCK_TICKS"
echo $(( 1000 + n ))
@@ -240,15 +256,16 @@ polls=$(cat "$d3/.poll" 2>/dev/null || echo 0)
[ "$got" = 0 ] && [ "$polls" = 3 ] && ok "a review arriving on the third poll is seen" \
|| bad "a review arriving on the third poll is seen" "exit $got after $polls polls, wanted 0 after 3"
# The timeout path: nothing ever arrives, so --wait 5 on a clock that ticks
# once per read must keep polling until the deadline, then report exit 1.
# The timeout path: nothing arrives, and on a clock that ticks once per read the
# count is exact: 1 poll, then one per tick until the deadline, so WAIT + 1.
WAIT=5
rm -f "$work/empty/.poll" "$work/ticks"
CLOCK_TICKS="$work/ticks" PATH="$clock:$PATH" PR_REVIEWS_FIXTURE_DIR="$work/empty" \
"$REVIEWS" 1 --wait 5 --interval 1 > "$work/out" 2>&1
"$REVIEWS" 1 --wait "$WAIT" --interval 1 > "$work/out" 2>&1
got=$?
polls=$(cat "$work/empty/.poll" 2>/dev/null || echo 0)
[ "$got" = 1 ] && [ "$polls" -ge 5 ] && ok "--wait keeps polling until its deadline" \
|| bad "--wait keeps polling until its deadline" "exit $got after $polls polls, wanted 1 after 5 or more"
[ "$got" = 1 ] && [ "$polls" = $((WAIT + 1)) ] && ok "--wait keeps polling until its deadline" \
|| bad "--wait keeps polling until its deadline" "exit $got after $polls polls, wanted 1 after $((WAIT + 1))"
# ---------------------------------------------------------------------------
# `gh api --paginate` emits one array PER PAGE, so a thread past the page size
scripts/pr-watch
+9 -2
View File
@@ -18,6 +18,7 @@ exit 2 the head moved during the wait, or is not --expect
exit 3 timed out with checks still running, or mergeability still UNKNOWN,
or still BLOCKED with every registered check green
exit 4 usage or environment error
exit 5 the PR is MERGED or CLOSED, so there is nothing left to wait for
EOF
exit 4
}
@@ -51,10 +52,10 @@ fetch() {
return
fi
if command -v gh >/dev/null 2>&1 && gh --version 2>/dev/null | grep -q '^gh version'; then
gh pr view "$PR" --json headRefOid,mergeable,mergeStateStatus,statusCheckRollup
gh pr view "$PR" --json state,headRefOid,mergeable,mergeStateStatus,statusCheckRollup
else
nix shell nixpkgs#gh --command gh pr view "$PR" \
--json headRefOid,mergeable,mergeStateStatus,statusCheckRollup
--json state,headRefOid,mergeable,mergeStateStatus,statusCheckRollup
fi
}
@@ -80,6 +81,12 @@ while :; do
echo "pr-watch: head moved $pinned -> $head during the wait" >&2
exit 2
fi
# A PR no longer open ends the wait (improvise-1qaa), but only after the head
# checks: a merge at a head never seen is PR #67 again (improvise-24lz).
pr_state="$(printf '%s' "$json" | jqf '.state')"
case "$pr_state" in
MERGED|CLOSED) echo "pr-watch: PR $PR is $pr_state, nothing to wait for" >&2; exit 5 ;;
esac
# total>0 AND nothing still running. "0 pending" alone reads identically for
# "all finished" and "none registered yet", which is how this exited instantly
scripts/pr-watch-test.sh
+36
View File
@@ -153,6 +153,42 @@ run_case "the needs-gap before the required check registers is not a failure" 0
gap_state "$work/gap-never" last.json MERGEABLE BLOCKED "$not_yet"
run_case "a required check that never registers is a timeout" 3 "$work/gap-never"
# improvise-9pfs. Here a failed REQUIRED check reports MERGEABLE/BLOCKED, not
# UNSTABLE. The table above holds only green rollups, so this red was unpinned.
gap_state "$work/blocked-red" last.json MERGEABLE BLOCKED \
'map(if .name == "Test and lint (host)" then .conclusion = "FAILURE" else . end)'
run_case "a failed required check under MERGEABLE/BLOCKED fails without waiting" 1 \
"$work/blocked-red" --timeout 0
# improvise-1qaa/29w2. Each ended fixture is a shape GitHub returned live (#189,
# #74, #46); the last is the one a waiting loop would sit on.
# $1 dir, $2 file, $3 head, $4 state, $5 mergeable, $6 mergeStateStatus
ended() {
mkdir -p "$1"
printf '{"headRefOid":"%s","state":"%s","mergeable":"%s","mergeStateStatus":"%s","statusCheckRollup":%s}\n' \
"$3" "$4" "$5" "$6" "$green_rollup" > "$1/$2"
}
for shape in "MERGED UNKNOWN UNKNOWN" "CLOSED CONFLICTING DIRTY" "CLOSED MERGEABLE BLOCKED"; do
set -- $shape
d="$work/ended-$1-$3"
ended "$d" last.json aaaaaaaaaaaa "$1" "$2" "$3"
run_case "a $1 PR at $2/$3 ends the watch as no longer open" 5 "$d" --timeout 5
polls="$(cat "$d/.poll" 2>/dev/null || echo 0)"
[ "$polls" = 1 ] && ok "a $1 PR at $2/$3 is read once, not waited on" \
|| bad "a $1 PR at $2/$3 is read once, not waited on (polls)" "$polls" 1
done
# improvise-24lz. A merge at a head the watcher never saw is the PR #67 hazard,
# so a moved head outranks a closed PR, with or without --expect.
state "$work/moved-merged" 1.json '"MERGEABLE"' '"UNSTABLE"' \
'[{"name":"build","status":"IN_PROGRESS","conclusion":""}]'
ended "$work/moved-merged" last.json bbbbbbbbbbbb MERGED UNKNOWN UNKNOWN
run_case "a head that moves and then merges is not reported as merged" 2 \
"$work/moved-merged" --timeout 60
ended "$work/merged-elsewhere" last.json bbbbbbbbbbbb MERGED UNKNOWN UNKNOWN
run_case "a PR merged at a head other than --expect is a head mismatch" 2 \
"$work/merged-elsewhere" --expect aaaa --timeout 5
# improvise-h3ni. The head must stay pinned across the UNKNOWN wait, or a
# verdict about b is reported as a verdict about a.
state "$work/unk-moved" 1.json '"UNKNOWN"' '"UNKNOWN"' "$green_rollup"
src/draw.rs
+218 -4
View File
@@ -99,11 +99,42 @@ pub fn run_tui(
Ok(())
}
/// One keypress in the TUI. A message lasts until the next key, so it is drawn
/// once and then gives way to the hint (improvise-cq3d, `context/code/tui.md`).
/// One keypress in the TUI. A message lasts until the next key (improvise-cq3d),
/// except while one editor stays open under it (su4u, sx7h). `context/code/tui.md`.
fn press(app: &mut App, key: key::KeyEvent) -> Result<()> {
let before = EditorSession::of(app);
let stale = app.view_state.status_msg.clone();
app.apply_effects(vec![Box::new(crate::ui::effect::SetStatus(String::new()))]);
app.handle_key(key)
app.handle_key(key)?;
let still_editing = before.is_some() && before == EditorSession::of(app);
if still_editing && app.view_state.status_msg.is_empty() && !stale.is_empty() {
app.apply_effects(vec![Box::new(crate::ui::effect::SetStatus(stale))]);
}
Ok(())
}
/// What one opening of a minibuffer sets, which is all the TUI loop can see of
/// it: equal before and after a key means the same editor (improvise-sx7h).
#[derive(Debug, PartialEq)]
struct EditorSession {
mode: AppMode,
cell: (usize, usize),
seed: Option<String>,
}
impl EditorSession {
fn of(app: &App) -> Option<Self> {
app.view_state.mode.minibuffer()?;
Some(Self {
mode: app.view_state.mode.clone(),
cell: app.cursor().selected,
seed: app
.view_state
.opened_editor
.as_ref()
.map(|o| o.seed.clone()),
})
}
}
// ── Key event conversion (TUI boundary) ──────────────────────────────────────
@@ -573,7 +604,7 @@ fn draw_welcome(f: &mut Frame, area: Rect) {
#[cfg(test)]
mod bottom_bar_tests {
use super::{draw_bottom_bar, minibuffer_line, press};
use super::{EditorSession, draw_bottom_bar, minibuffer_line, press};
use crate::command::key::{KeyCode, KeyEvent, KeyModifiers};
use crate::ui::app::{App, AppMode};
use crate::workbook::Workbook;
@@ -635,6 +666,189 @@ mod bottom_bar_tests {
);
}
/// A refusal that keeps the user in a minibuffer stands while they fix it, and
/// goes when they leave (improvise-su4u). `context/test-evidence/tui.md`.
#[test]
fn a_minibuffer_refusal_stands_while_the_user_fixes_it() {
let key = |code| KeyEvent::new(code, KeyModifiers::NONE);
let refused = || {
let mut app = App::new(Workbook::new("T"), None);
app.view_state.formula_panel_open = true;
app.view_state.mode = AppMode::FormulaPanel;
press(&mut app, key(KeyCode::Char('n'))).unwrap();
for c in "Total = (".chars() {
press(&mut app, key(KeyCode::Char(c))).unwrap();
}
press(&mut app, key(KeyCode::Enter)).unwrap();
assert!(
matches!(app.view_state.mode, AppMode::FormulaEdit { .. }),
"a bad formula keeps the editor open"
);
assert!(bottom_row(&app, 240).contains("Formula error"));
app
};
let mut app = refused();
press(&mut app, key(KeyCode::Backspace)).unwrap();
let row = bottom_row(&app, 240);
assert!(row.contains("Total = "), "still editing: {row:?}");
assert!(
row.contains("Formula error"),
"the refusal vanished on the first correcting key: {row:?}"
);
let first = app.view_state.status_msg.clone();
press(&mut app, key(KeyCode::Enter)).unwrap();
let second = app.view_state.status_msg.clone();
assert_ne!(second, first, "a new refusal names the new text");
assert!(
bottom_row(&app, 240).contains(second.lines().next().unwrap_or_default()),
"a key's own message must beat the stale one"
);
press(&mut app, key(KeyCode::Char('1'))).unwrap();
press(&mut app, key(KeyCode::Enter)).unwrap();
assert!(!matches!(app.view_state.mode, AppMode::FormulaEdit { .. }));
let row = bottom_row(&app, 240);
assert!(
!row.contains("Formula error"),
"the refusal outlived its fix: {row:?}"
);
let mut app = refused();
press(&mut app, key(KeyCode::Esc)).unwrap();
let row = bottom_row(&app, 240);
assert!(
!row.contains("Formula error"),
"the refusal outlived leaving the editor: {row:?}"
);
}
/// Region{North, South}, two measures, one cell: the reviewer's records fixture
/// for improvise-sx7h, plus a second row for the plain grid's Enter.
fn one_record() -> App {
use crate::model::cell::{CellKey, CellValue};
let mut wb = Workbook::new("T");
wb.add_category("Region").unwrap();
wb.add_item("Region", "North").unwrap();
wb.add_item("Region", "South").unwrap();
wb.add_item("_Measure", "Cost").unwrap();
wb.add_item("_Measure", "Revenue").unwrap();
wb.model.set_cell(
CellKey::new(vec![
("Region".into(), "North".into()),
("_Measure".into(), "Revenue".into()),
]),
CellValue::Number(1.0),
);
App::new(wb, None)
}
/// A commit that re-opens the editor on another cell starts another editor, so
/// a message from the last one must not follow it (improvise-sx7h). Evidence file.
#[test]
fn a_message_does_not_follow_the_editor_onto_the_next_cell() {
let key = |code| KeyEvent::new(code, KeyModifiers::NONE);
let ch = |app: &mut App, c: char| press(app, key(KeyCode::Char(c))).unwrap();
// Records: `o` says it added a row and opens an editor on it.
let mut app = one_record();
ch(&mut app, 'R');
ch(&mut app, 'o');
let added = app.view_state.status_msg.clone();
assert!(!added.is_empty(), "o must say what it did");
assert!(matches!(
app.view_state.mode,
AppMode::RecordsEditing { .. }
));
ch(&mut app, '5');
assert!(
bottom_row(&app, 240).contains(&added),
"typing in the same editor keeps the message"
);
let cell = app.cursor().selected;
press(&mut app, key(KeyCode::Tab)).unwrap();
assert!(matches!(
app.view_state.mode,
AppMode::RecordsEditing { .. }
));
assert_ne!(app.cursor().selected, cell, "Tab moved the editor");
let row = bottom_row(&app, 240);
assert!(
!row.contains(&added),
"the message followed the editor to the next cell: {row:?}"
);
// Records: a second `o` on the blank row is refused, and filling it in ends it.
let mut app = one_record();
ch(&mut app, 'R');
ch(&mut app, 'o');
press(&mut app, key(KeyCode::Esc)).unwrap();
ch(&mut app, 'o');
let refusal = app.view_state.status_msg.clone();
assert!(refusal.contains("already here"), "{refusal:?}");
ch(&mut app, '5');
assert!(bottom_row(&app, 240).contains("already here"));
press(&mut app, key(KeyCode::Tab)).unwrap();
let row = bottom_row(&app, 240);
assert!(
!row.contains("already here"),
"the refusal outlived the Tab that answered it: {row:?}"
);
// A plain grid: Enter commits and re-opens the editor one row down.
let mut app = one_record();
app.cursor_mut().selected = (0, 0);
ch(&mut app, 'i');
assert!(matches!(app.view_state.mode, AppMode::Editing { .. }));
app.view_state.status_msg = "a note about this cell".into();
ch(&mut app, '7');
assert!(bottom_row(&app, 240).contains("a note about this cell"));
let cell = app.cursor().selected;
press(&mut app, key(KeyCode::Enter)).unwrap();
assert!(matches!(app.view_state.mode, AppMode::Editing { .. }));
assert_ne!(app.cursor().selected, cell, "Enter moved the editor");
let row = bottom_row(&app, 240);
assert!(
!row.contains("a note about this cell"),
"the note followed the editor down a row: {row:?}"
);
// The last column: Tab commits and re-opens the editor on the SAME cell, so
// only the new seed says it is another editor.
let mut app = one_record();
let last = app.layout.col_count() - 1;
app.cursor_mut().selected = (0, last);
ch(&mut app, 'i');
app.view_state.status_msg = "a note about this cell".into();
ch(&mut app, '7');
press(&mut app, key(KeyCode::Tab)).unwrap();
assert!(matches!(app.view_state.mode, AppMode::Editing { .. }));
assert_eq!(app.cursor().selected, (0, last), "Tab could not advance");
let row = bottom_row(&app, 240);
assert!(
!row.contains("a note about this cell"),
"the note outlived the commit on the same cell: {row:?}"
);
}
/// Equal modes are the test, not equal variants: an editor adding to another
/// category is another editor (improvise-sx7h). Evidence file.
#[test]
fn an_editor_on_another_category_is_another_editor() {
let mut app = App::new(Workbook::new("T"), None);
let item_add = |category: &str| AppMode::item_add(category.into());
app.view_state.mode = item_add("Region");
let region = EditorSession::of(&app);
assert!(region.is_some(), "ItemAdd is a text-entry mode");
app.view_state.mode = item_add("Month");
assert_ne!(region, EditorSession::of(&app));
app.view_state.mode = item_add("Region");
assert_eq!(
region,
EditorSession::of(&app),
"and the same one is the same"
);
}
/// The wizard overlay draws correct per-step hints; the status bar stacked a
/// second static table under it (improvise-rr8n) — `context/test-evidence/tui.md`.
#[test]