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

Merge gh/main into fold-memo/metf-opbo (#291)

Browse Source 
One conflict, context/test-evidence/command.md: both sides appended at EOF.
Kept both entries, the branch's first.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
Edward Langley
2026-09-23 23:10:49 -07:00
parent 5b7e0223aa 666de19963
commit d6f74847f8
40 changed files with 3009 additions and 537 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
+120 -168
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,52 @@ 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
# scripts/web-toolchain-versions.env stays the one home for the pins, so
# the release capture (scripts/build-web-release.sh, improvise-oujb)
# provisions the same versions. CI no longer installs them — the dev
# shell supplies wasm-pack and node — so this checks the shell against
# the pins instead: a nixpkgs bump that moves either fails here, by name.
# wasm-pack runs its own bundled `wasm-opt`, which is why
# improvise-wasm-client's Cargo.toml names `--enable-bulk-memory`.
- name: The dev shell supplies the pinned web toolchain
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"
. scripts/web-toolchain-versions.env
test "$(wasm-pack --version)" = "wasm-pack ${WASM_PACK_VERSION#v}"
test "$(node -p 'process.versions.node.split(".")[0]')" = "$NODE_VERSION"
wasm-pack --version
node --version
# wasm-pack uses the shell's wasm-bindgen only at Cargo.lock's exact
# version; otherwise it cargo-installs one (71s) — so a lock bump
# must move flake.nix's wasmBindgenCli too, and this says so.
lock="$(awk '/^name = "wasm-bindgen"$/ {getline; gsub(/[^0-9.]/, ""); print}' Cargo.lock)"
test "$(wasm-bindgen --version)" = "wasm-bindgen $lock" || {
echo "flake.nix wasmBindgenCli is $(wasm-bindgen --version); Cargo.lock wants $lock" >&2
exit 1
}
# 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"
# 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
shell: bash
run: |
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
shell: bash
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 +133,29 @@ 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
shell: bash
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
shell: bash
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 +163,47 @@ 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
# Plain bash, not the dev shell: this step needs nix and nothing else.
- name: nix build .#improvise
shell: bash
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
shell: bash
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,23 +218,23 @@ 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
run: ./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).
@@ -301,20 +267,14 @@ 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
shell: bash
run: nix build --print-build-logs .#checks.x86_64-linux.harness-lint
# ---------------------------------------------------------------------------
# The browser harness (improvise-vvl3, absorbing improvise-xf1).
@@ -335,37 +295,25 @@ 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
# No browser install step: the dev shell points PLAYWRIGHT_BROWSERS_PATH
# at nixpkgs' Chromium, Firefox and WebKit, since Playwright's own
# downloads cannot run on NixOS. All three, because improvise-xf1's
# acceptance criterion names Chrome, Firefox and Safari and
# `createSyncAccessHandle` is implemented separately by each engine.
# WebKit's OPFS is absent today; the spec skips on a runtime probe.
# @playwright/test must equal nixpkgs' playwright-driver version, or it
# looks for browser builds that are not there; this says which.
- name: Playwright matches the dev shell's browsers
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
run: |
npx playwright --version
ls "$PLAYWRIGHT_BROWSERS_PATH"
# 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 +337,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 +384,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:
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
+91 -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
@@ -3551,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
@@ -3741,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/tooling.md
+29 -5
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
@@ -645,10 +667,12 @@ one place, with no per-invocation nix cost:
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:
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.
1. **Pins a bash ≥ 5.1.** 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.1 and
no nix is **exit 3** — a refusal, never a quiet run under 3.2. The floor is
5.1 and not 5 because `pr-reviews-test.sh` uses `wait -n -p`, and `-p`
arrived in 5.1. Under 5.0 it is an invalid option (improvise-fd0j).
2. **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
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
+107 -1
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
@@ -2105,7 +2144,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 +4620,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
@@ -5117,3 +5190,36 @@ 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`.
### 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
+197 -8
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
@@ -5346,11 +5354,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
@@ -5382,7 +5446,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
@@ -5921,8 +5988,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.
@@ -5943,6 +6012,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
@@ -6306,3 +6441,57 @@ shipped build.
Each rival re-derives one half's chain, so the count moves off 18. The file
was restored after each and compared byte for byte.
## `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/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
+201
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,75 @@ 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.
context/test-evidence/tui.md
+112
View File
@@ -490,6 +490,61 @@ in a scratch repository and asserts the walk lists it. All four were red
against the `.gitignore` walk. The account, including the rivals, is under
`every_shipped_document_loads` above.
### a_scratch_repo_ignores_an_inherited_git_dir
improvise-53zz. The scratch repositories inherited the caller's git
environment, and two parts of it change what `git -C <scratch>` does.
**The repo-local variables beat `-C`.** A hook running in a linked worktree
exports `GIT_DIR` (and, during a commit, `GIT_INDEX_FILE`). Under those,
`git -C <scratch> init` re-initialised the *outer* repository and `git add`
staged the scratch files into the *outer* index, so the test suite wrote into
the checkout that ran it. Three of the four shape tests still passed that way,
because `ls-files` read the same wrong index that `add` had just written. So a
green run was no evidence the walk had looked at the scratch repository.
The fix is one helper, `git(root)`, which removes every name that
`git rev-parse --local-env-vars` prints before adding `-C`. It asks git rather
than hard-coding a list, so the list follows the installed git. Both
`shipped_documents` and the scratch helpers go through it, so the production
walk ignores an inherited `GIT_DIR` too.
The test must set the variable, and a test thread cannot set it without
setting it for every other thread. So the test re-runs this same binary as a
child, restricted to `a_single_file_un_ignore_is_walked`, with `GIT_DIR` and
`GIT_INDEX_FILE` naming a throwaway outer repository. It asserts two things:
the child passed, and the outer index is still empty. Before the fix the second
assertion failed with `the outer index gained: .gitignore`.
Rivals, each run against the fixed file:
- Remove only `GIT_DIR`: red, because `GIT_INDEX_FILE` still sends the add to
the outer index.
- Remove only `GIT_INDEX_FILE`: red, because `GIT_DIR` does.
- Rename the child test so the child runs nothing: red on
`… did not run`. `rerun_with_env` asserts `running 1 test`, because
libtest exits 0 when a filter matches nothing, and both of these tests would
then pass without running anything.
### a_scratch_repo_ignores_the_users_global_and_system_excludes
improvise-53zz. The scratch repositories also read the user's own git config. A
`*.gz` line in a global `core.excludesFile` made `git add` refuse
`docs/model.improv.gz`, so `a_gzipped_document_is_walked` failed on that
machine and passed on others. `run_git` now sets `GIT_CONFIG_GLOBAL=/dev/null`
and `GIT_CONFIG_NOSYSTEM=1`. `shipped_documents` does not need them: `ls-files`
reads the index, and excludes never apply to a tracked file.
The test writes a config whose `excludesFile` holds `*.gz`, and re-runs the
gzip shape test in a child once under `GIT_CONFIG_GLOBAL` and once under
`GIT_CONFIG_SYSTEM`. Before the fix the global run failed at `git add`.
Rivals:
- Drop `GIT_CONFIG_GLOBAL=/dev/null`: red, in the global run.
- Drop `GIT_CONFIG_NOSYSTEM=1`: red, in the system run. The system scope is
there so that this half of the fix has a witness at all.
---
## `tests/web_demo_model.rs`
@@ -946,3 +1001,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/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/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/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-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/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/resolve.rs
+195 -56
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(
@@ -786,9 +814,7 @@ fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option<Vec<Command>> {
"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"))]),
@@ -993,9 +1019,8 @@ 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));
// The new cell is a different cell, so a stale cache is correct here.
commands.extend(reopened_at(state, nr, nc, edit_mode));
Some(commands)
}
@@ -1018,20 +1043,40 @@ 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) -> 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 => {
opened_on(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)]
@@ -2402,7 +2447,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 +2476,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 +2709,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]
@@ -3454,6 +3548,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() {
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
+73 -26
View File
@@ -2091,36 +2091,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 {
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
+85 -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,76 @@
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; CI checks that it does.
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). Must match WASM_PACK_VERSION in
# scripts/web-toolchain-versions.env; CI checks that it does.
pkgs.wasm-pack
wasmBindgenCli
pkgs.binaryen
# NODE_VERSION's major, same check. The js and browser jobs ran 22
# while `build` ran the pinned 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 +116,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 +123,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
+22 -3
View File
@@ -133,14 +133,33 @@ 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" \
# utilbin carries only the utilities the runner itself needs, and no nix, so
# the fake bash is the only candidate and the nix fallback is absent. Linked
# one by one rather than taking /usr/bin:/bin, which on NixOS hold only `env`
# and `sh` -- the runner died on `dirname` there, exit 1 instead of 3.
mkdir -p "$work/utilbin"
for tool in dirname; do
ln -sf "$(command -v "$tool")" "$work/utilbin/$tool"
done
env PATH="$work/fakebin:$work/utilbin" 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"
# 5b. The floor is 5.1, not 5: `wait -n -p` is 5.1 (improvise-fd0j). The 5.1
# control proves the 5.0 refusal is the version and not the fake itself.
for v in "5 0:3:a bash 5.0 with no nix is exit 3, not 5.x" \
"5 1:0:a bash 5.1 is accepted"; do
printf '#!/bin/sh\ncase "$*" in *BASH_VERSINFO*) echo %s; exit 0 ;; esac\nexit 0\n' \
"${v%%:*}" > "$work/fakebin/bash"
want="${v#*:}"; name="${want#*:}"; want="${want%%:*}"
env PATH="$work/fakebin:/usr/bin:/bin" HOME="$HOME" \
"${BASH:-bash}" "$d/checkers.sh" any > "$work/out" 2>&1
got=$?
[ "$got" = "$want" ] && ok "$name" || bad "$name" "exit $got, wanted $want"
done
# ---------------------------------------------------------------------------
# 6. A named suite that does not exist is a usage error, not a silent no-op.
d="$(sandbox missing)"
scripts/checkers.sh
+10 -7
View File
@@ -5,7 +5,7 @@
#
# It does three things a bare `./scripts/X-test.sh` does not:
#
# 1. pins a bash >= 5, taken from nix when the ambient one is older, so the
# 1. pins a bash >= 5.1, 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`
# exports `name`, `out`, `system`, `shell`, `builder` and two dozen more
@@ -30,7 +30,7 @@ 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 3 no bash >= 5.1 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,14 +46,17 @@ 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
# Prints the absolute path of a bash >= 5.1, 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
local candidate major minor
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
read -r major minor < <("$candidate" -c 'echo "${BASH_VERSINFO[0]} ${BASH_VERSINFO[1]}"' \
2>/dev/null || echo 0 0)
if [ "$(( ${major:-0} * 100 + ${minor:-0} ))" -ge 501 ] 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
@@ -122,7 +125,7 @@ main() {
local pinned; pinned="$(find_modern_bash)"
if [ -z "$pinned" ]; then
echo 'checkers: no bash >= 5 on PATH and nix cannot supply one.' >&2
echo 'checkers: no bash >= 5.1 on PATH and nix cannot supply one.' >&2
echo ' Enter the dev shell (direnv allow / nix develop) and retry.' >&2
return 3
fi
scripts/pr-reviews-test.sh
+22 -6
View File
@@ -5,10 +5,25 @@
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. /bin/sleep, because some cases put a no-op `sleep` first on PATH.
BOUND=20
bounded_reviews() {
"$here/pr-reviews" "$@" &
local pid=$! dog done rc
/bin/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)); }
@@ -240,15 +255,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]
tests/shipped_documents_load.rs
+85 -6
View File
@@ -6,9 +6,7 @@ use std::path::{Path, PathBuf};
/// Every tracked `.improv` and `.improv.gz` under `root`. Outside a checkout this
/// panics rather than answering empty (improvise-c0aj).
fn shipped_documents(root: &Path) -> Vec<PathBuf> {
let out = std::process::Command::new("git")
.arg("-C")
.arg(root)
let out = git(root)
.args(["ls-files", "-z", "--", "*.improv", "*.improv.gz"])
.output()
.expect("git runs: this test reads the index, so it needs a checkout");
@@ -50,6 +48,21 @@ fn every_shipped_document_loads() {
}
}
/// `git -C root`, with every repo-local variable a caller exported removed,
/// since `GIT_DIR` and friends override `-C` (improvise-53zz).
fn git(root: &Path) -> std::process::Command {
let names = std::process::Command::new("git")
.args(["rev-parse", "--local-env-vars"])
.output()
.expect("git runs");
let mut git = std::process::Command::new("git");
for name in String::from_utf8(names.stdout).expect("UTF-8").lines() {
git.env_remove(name);
}
git.arg("-C").arg(root);
git
}
/// A scratch repository, so each way of shipping a document is set up for real.
fn scratch_repo() -> tempfile::TempDir {
let dir = tempfile::tempdir().expect("a temp dir");
@@ -58,9 +71,9 @@ fn scratch_repo() -> tempfile::TempDir {
}
fn run_git(root: &Path, args: &[&str]) {
let status = std::process::Command::new("git")
.arg("-C")
.arg(root)
let status = git(root)
.env("GIT_CONFIG_GLOBAL", "/dev/null")
.env("GIT_CONFIG_NOSYSTEM", "1")
.args(args)
.status()
.expect("git runs");
@@ -137,3 +150,69 @@ fn a_force_added_document_is_walked() {
run_git(root, &["add", "-f", "secret/forced.improv"]);
assert!(walked(root, "secret/forced.improv"));
}
/// Runs one of this binary's own tests in a child process with `env` added,
/// since the environment is what is under test and threads share ours.
fn rerun_with_env(test: &str, env: &[(&str, &Path)]) -> std::process::Output {
let mut child = std::process::Command::new(std::env::current_exe().expect("test binary"));
child.args([test, "--exact", "--nocapture"]);
for (name, value) in env {
child.env(name, value);
}
let out = child.output().expect("the test binary runs");
let stdout = String::from_utf8_lossy(&out.stdout);
assert!(
stdout.contains("running 1 test"),
"{test} did not run: {stdout}"
);
out
}
/// improvise-53zz: a hook in a linked worktree exports `GIT_DIR`, which beats
/// `-C`. Evidence: `context/test-evidence/tui.md`.
#[test]
fn a_scratch_repo_ignores_an_inherited_git_dir() {
let outer = scratch_repo();
let dot_git = outer.path().join(".git");
let out = rerun_with_env(
"a_single_file_un_ignore_is_walked",
&[
("GIT_DIR", &dot_git),
("GIT_INDEX_FILE", &dot_git.join("index")),
],
);
assert!(
out.status.success(),
"the shape test fails under an inherited GIT_DIR: {}",
String::from_utf8_lossy(&out.stdout)
);
let staged = git(outer.path())
.args(["ls-files"])
.output()
.expect("git runs");
assert!(
staged.stdout.is_empty(),
"the outer index gained: {}",
String::from_utf8_lossy(&staged.stdout)
);
}
/// improvise-53zz: a user's `*.gz` exclude refused the gzip shape test's `add`.
/// Evidence: `context/test-evidence/tui.md`.
#[test]
fn a_scratch_repo_ignores_the_users_global_and_system_excludes() {
let home = tempfile::tempdir().expect("a temp dir");
let excludes = home.path().join("ignore");
std::fs::write(&excludes, "*.gz\n").expect("writes");
let config = home.path().join("gitconfig");
let text = format!("[core]\n\texcludesFile = {}\n", excludes.display());
std::fs::write(&config, text).expect("writes");
for scope in ["GIT_CONFIG_GLOBAL", "GIT_CONFIG_SYSTEM"] {
let out = rerun_with_env("a_gzipped_document_is_walked", &[(scope, &config)]);
assert!(
out.status.success(),
"the gzip shape test fails under a `*.gz` exclude in {scope}: {}",
String::from_utf8_lossy(&out.stdout)
);
}
}