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

Merge gh/main (cc95cfbf) into fix/8wy9-restore-rebuild

Browse Source 
# Conflicts:
#	context/test-evidence/command.md
This commit is contained in:
Edward Langley
2026-09-23 23:22:07 -07:00
parent 86b3d72c70 cc95cfbfdd
commit 3edb63022c
48 changed files with 4115 additions and 630 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:
benches/grid_projection.rs
+114 -2
View File
@@ -54,6 +54,102 @@ fn dense_app(regions: usize, products: usize, measures: usize) -> App {
app
}
/// Collapsed groups on the row axis and a formula chain on the column axis, so
/// every fold cell and every total re-derives the chain (improvise-h6z1).
fn chain_app(regions: usize, products: usize) -> App {
let mut wb = Workbook::new("Chain");
wb.add_category("Region").unwrap();
wb.add_category("Product").unwrap();
for r in 0..regions {
wb.add_item_in_group("Region", format!("Region{r}"), format!("G{}", r / 10))
.unwrap();
}
for p in 0..products {
wb.add_item("Product", format!("Product{p}")).unwrap();
}
wb.add_item("_Measure", "Revenue").unwrap();
let chain = [
"F1 = Revenue * 2",
"F2 = F1 + Revenue",
"F3 = F2 + F1",
"F4 = F3 + F2",
"F5 = F4 / F1",
];
for text in chain {
let formula = improvise::formula::parse_formula(text, "_Measure").unwrap();
wb.model.add_formula(formula);
}
{
let v = wb.active_view_mut();
v.set_axis("Region", Axis::Row);
v.set_axis("Product", Axis::Row);
v.set_axis("_Measure", Axis::Column);
for g in 0..regions.div_ceil(10) {
v.toggle_group_collapse("Region", &format!("G{g}"));
}
}
let mut n = 0.0f64;
for r in 0..regions {
for p in 0..products {
n += 1.0;
let key = CellKey::new(vec![
("Region".to_string(), format!("Region{r}")),
("Product".to_string(), format!("Product{p}")),
("_Measure".to_string(), "Revenue".to_string()),
]);
wb.model.set_cell(key, CellValue::Number(n));
}
}
let mut app = App::new(wb, None);
app.rebuild_layout();
app
}
/// Days in collapsed groups of `per_group` on Row, with Product: every row is a
/// fold over a few days, never the whole category (improvise-jq0m).
fn grouped_app(days: usize, per_group: usize, products: usize, measures: usize) -> App {
let mut wb = Workbook::new("Grouped");
wb.add_category("Day").unwrap();
wb.add_category("Product").unwrap();
for d in 0..days {
let group = format!("G{}", d / per_group);
wb.add_item_in_group("Day", format!("Day{d}"), group)
.unwrap();
}
for p in 0..products {
wb.add_item("Product", format!("Product{p}")).unwrap();
}
for m in 0..measures {
wb.add_item("_Measure", format!("Measure{m}")).unwrap();
}
{
let v = wb.active_view_mut();
v.set_axis("Day", Axis::Row);
v.set_axis("Product", Axis::Row);
v.set_axis("_Measure", Axis::Column);
for g in 0..days.div_ceil(per_group) {
v.toggle_group_collapse("Day", &format!("G{g}"));
}
}
let mut n = 0.0f64;
for d in 0..days {
for p in 0..products {
for m in 0..measures {
n += 1.0;
let key = CellKey::new(vec![
("Day".to_string(), format!("Day{d}")),
("Product".to_string(), format!("Product{p}")),
("_Measure".to_string(), format!("Measure{m}")),
]);
wb.model.set_cell(key, CellValue::Number(n));
}
}
}
let mut app = App::new(wb, None);
app.rebuild_layout();
app
}
fn whole_grid(app: &App) -> Viewport {
Viewport {
row_offset: 0,
@@ -105,7 +201,10 @@ fn time_widths(iters: u32, app: &App) -> Duration {
}
fn case(label: &str, regions: usize, products: usize, measures: usize, iters: u32) {
let app = dense_app(regions, products, measures);
report(label, dense_app(regions, products, measures), iters);
}
fn report(label: &str, app: App, iters: u32) {
let rows = app.layout.row_count();
let cols = app.layout.col_count();
let cells = rows * cols;
@@ -120,9 +219,12 @@ fn case(label: &str, regions: usize, products: usize, measures: usize, iters: u3
true => "UNSTABLE".to_string(),
false => format!("{share:.0}%"),
};
// The TUI pays both on every keypress: widths in `cmd_context`, the grid in draw.
let keypress = full + widths;
println!(
"{label:<22} {rows:>5} x {cols:>3} = {cells:>7} cells whole-grid {full:>11.3?} \
windowed {windowed:>11.3?} ratio {ratio:>4.1}x col_widths {widths:>11.3?} ({share} of the frame)"
windowed {windowed:>11.3?} ratio {ratio:>4.1}x col_widths {widths:>11.3?} ({share} of the frame) \
keypress {keypress:>11.3?}"
);
}
@@ -130,7 +232,17 @@ fn main() {
println!("project::grid_of, one call = one TUI frame's grid cache");
println!("a 60fps frame budget is 16.7ms; the TUI redraws on each keypress\n");
case("typical", 20, 5, 4, 200);
case("1.2k cells", 60, 5, 4, 100);
case("2.4k cells", 60, 10, 4, 100);
case("4.8k cells", 60, 20, 4, 50);
case("large", 60, 20, 8, 50);
case("18k cells (gotcha 41)", 150, 20, 6, 20);
case("pathological", 300, 30, 10, 5);
println!("\nfolded rows and a five-formula chain (improvise-h6z1)\n");
report("folded chain ~18k", chain_app(1000, 30), 5);
report("folded chain ~90k", chain_app(5000, 30), 3);
println!("\ncollapsed small groups on the row axis (improvise-jq0m)\n");
report("364 days in weeks", grouped_app(364, 7, 20, 6), 5);
report("364 days in pairs", grouped_app(364, 2, 20, 6), 5);
report("1000 days in pairs", grouped_app(1000, 2, 10, 4), 3);
}
context/code/clients.md
+50 -3
View File
@@ -516,6 +516,21 @@ seeds the `edit` buffer from the cell it landed on. The trailing `clear-buffer`
the keymap welds to every commit then empties that buffer again. That ordering
looks wrong and is exactly what the TUI does; mirroring it is the point.
The re-entry is `reopened_at`, and it declines where the host's
`EnterEditAtCursor` declines (improvise-enz6). The host asks
`core::uneditable` of its layout. The browser asks the landing cell's projected
`CellId`, which is the same answer, because `cell_id` is what fills it. Two
limits follow from asking the cache:
- **The wording differs on a fold.** The host can tell a fold from any other
aggregate and names the fold. The browser sees only `CellId::Aggregate`, so it
gives the host's generic aggregate text. `lockstep.rs` compares the mode, the
cursor and the buffers here, never the status text.
- **A landing past the window opens anyway.** The cache holds only the
announced window, so a `Down` from the window's last row lands on a cell with
no projected id. The browser then opens the editor, where the host does not.
No write can follow it: once the host has projected that cell,
`record_at_cursor` finds no record there and the commit composes nothing.
`GridMove` is a hand-spelled mirror of `navigation.rs`'s `MoveKind`, because the
command crate is not on a browser's dependency list and never will be. The
*keys* that reach it are no longer spelled twice. `scrolled_to` mirrors
@@ -578,11 +593,43 @@ immutable `CmdContext`; the commands are then folded in order.
### What still needs the host
`the_command_names_that_still_need_the_host_are_these` names them, and
`Host::handle` answers a `Key` frame with an empty vec, so each one is a binding
that **does nothing at all** in a browser
`resolve::NEEDS_THE_HOST` names them, and
`the_command_names_that_still_need_the_host_are_these` drives each through the
key that reaches it. `Host::handle` answers a `Key` frame with an empty vec, so
each one is a binding that **does nothing at all** in a browser
(`doctrine(an_upstream_key_is_a_black_hole)`).
**The help overlay marks them, and it marks rather than omits** (improvise-uln5).
The overlay is shared content, and before this the browser painted every row
plainly, including six that do nothing there. That is worse than a gap:
improvise-koh6 showed that the keys typed after a dead one land on live Normal
bindings, and `i` opens an editor. The browser's `RenderHelpRow` now draws a row
whose command is on the list as `dim`, and appends "(not in the browser yet)"
to its description. It does the same for every ex-command row while `:` opens a
command line this client cannot run (`resolve::runs_the_command_line`, which is
`stranding`'s answer, not a second one).
Marking was chosen over omitting for three reasons:
- **Both front ends draw the same rows.** A page with rows missing is a second
layout of the same content.
- **The feature stays findable.** The row still says it exists in the terminal.
- **It needs no change in `web/`.** `dim` is a kind the page already paints.
The TUI is unaffected, because the marking lives in the browser's renderer.
**One list, held from both sides.** `NEEDS_THE_HOST` is production data because
the renderer needs it. Two tests hold it to what `resolve` actually does:
- `the_command_names_that_still_need_the_host_are_these` fails if a name on it
resolves locally.
- `every_help_row_the_browser_cannot_run_is_one_it_marks` fails if a help row
is silent in the browser without being on the list, or on the list without
being silent. It also fails if a *hint line* ever names a listed command,
because the status bar is never marked.
`every_advertised_command_answers_in_its_own_mode` therefore no longer restates
the black holes. It skips a silent row whose command is on the list, because
the other test owns it.
**An upstream key that the app also ADVERTISES is a different severity, and it
had its own class of defect** (improvise-rmvx). A key nobody offers doing
nothing is a gap; a key named in the mode's own hint line doing nothing reads as
context/code/command.md
+225 -13
View File
@@ -609,6 +609,47 @@ the client was told nothing at all.
`AddItem` case the same way and found none, so `AddItem` does not stop either.
An abort is added where a later effect depends on the failed one, never for
symmetry.
- **The `|| last_batch_aborted()` arm is kept, and the invariant it guards is
pinned (improvise-we1a).** Every model effect that aborts today also refuses,
so deleting the arm left the workspace green: nothing tested it. There were
two ways to close that. One was to make an abort without a refusal
impossible to write. The other, taken here, was to test the invariant with
the arm left in place. `every_failing_model_effect_that_aborts_also_refuses`
applies each failing `ModelCommand`'s own effects to an `App`, bypassing
`Host`, and asserts it refused. It also pins which cases abort, so it cannot
pass on a table that aborts nothing. The wire table cannot hold this
invariant, because the arm itself rejects a frame that aborted without
refusing. With AddCategory's arm reverted to status-only, the wire table
stays green. The option not taken is right for today's rules and wrong for
the future:
- `AbortChain` aborts without refusing, correctly. It is `ViewOnly`, and it
stops a chain whose premise no longer holds, which is not an error. A
type-level rule of "every abort refuses" would first have to exempt it.
- Making it refuse would change which `:` lines the headless CLI reports as
failures.
- Keeping the raw `abort_effects` setter away from `Model` effects alone would
be a new visibility split, and today no effect needs it.
So the arm stays, as defence for a future `Model` effect that aborts on
something other than an error.
**The table alone did not hold the invariant (improvise-lasd).** It lists
failing commands, so an abort at a site it does not list got past it. With
`on_view`'s missing-view branch changed to abort without refusing, the
workspace stayed green, and only the arm caught it. So `App::apply_effects`
now carries the rule itself. When the abort flag is set after a `Model`
effect, a `debug_assert!` requires that the batch has refused. `ViewOnly`
effects are exempt, so `AbortChain` is untouched. That answers the objection
above: the rule is on `EffectKind`, not a new visibility split.
- **Coverage:** it covers every model effect that any test drives to an
abort, listed or not. Under the `on_view` rival it fails two existing
tests.
- **Where it stops:** it is a debug assertion, so release builds rely on the
arm. It reads the batch-wide `batch_refused`, so a model effect that aborts
without refusing after an earlier effect in the same batch has refused
still gets past it.
- **Why the table stays:** it drives every failing command, and so it
exercises the assertion.
In a client's frame, any refusal becomes `Rejected(MutationRefused)`. The frame
is undone whole, and the client is told, which is the channel the narrated
@@ -637,12 +678,13 @@ Consequences:
suffices. So each `refuse_and_stop` site needs a key-path assertion for its
refusal as well as one for its abort. The abort is
`a_failed_load_or_import_does_not_run_the_quit_after_it` (improvise-514o).
Three sites are not in the key-path table (improvise-rtud). The quit test's
"setup:" assertion holds the refusal of `load` and `import-json absent.json`.
Every `refuse_and_stop` site now has a row in
`every_refusal_site_refuses_its_failure_and_only_its_failure`. The last three
were added by improvise-we1a: `load`, `import-json absent.json`, and
`AddFormula`'s parse arm. Before that, only indirect assertions held them.
The quit test's "setup:" assertion held `load` and `import-json absent.json`.
`a_formula_that_does_not_parse_keeps_the_user_in_the_editor_with_their_text`
holds `AddFormula`'s parse arm, which has no case in the quit test. The other
five import arms are in
`every_refusal_site_refuses_its_failure_and_only_its_failure`.
held the parse arm (improvise-rtud). Those assertions remain.
Each case in that table has a succeeding twin, so a site that refused
unconditionally would fail too. That matters because these are exactly the
lines a merge conflict could silently drop.
@@ -678,6 +720,15 @@ The effect itself never inspects `app.view_state.mode`: the mode is decided
statically by whoever invoked it. An earlier version branched on
`app.view_state.mode.is_records()` and got it wrong mid-batch.
It opens nothing on a cell `i` declines, and says why (improvise-enz6). The
question is `cmd::core::uneditable`, the one `committable_cell_key` asks. A
fold or another aggregate, or a formula's result, gets its refusal text in
the status line. The mode stays where the batch put it. `CommitAndAdvance`
already dropped to normal at its head (gotcha 21), so the user lands on the
fold in normal mode. It is a status line and not `App::refuse`, because the
commit before it succeeded. A `commit-cell-edit` script line whose advance lands on a
fold must not report its write as refused.
### Which effects reset the cursor, and why it is decided one at a time
Swapping the axes renumbers every row and column, so the old indices name
@@ -1753,6 +1804,140 @@ frames at 60fps, and the remaining cost is one genuine evaluation per cell. The
next lever is caching *across* frames, which needs an invalidation story the
`record_versions`/`layout_version` tiers could plausibly supply.
**The totals left the walk (improvise-wm20).** A column's total used to be the
walk's running sum of its cells, which is wrong for everything except a stored
measure under Sum or Count. It gave a ratio column the sum of its rows' ratios,
an Avg measure the sum of its rows' means, and a text column `0.00`. The user's
rule is that **a column's total is its cell as if every drawn row were one
fold**. `GridLayout::column_total` asks `Model::evaluate_spanning` for the
column key with the row categories spanned, so a formula is evaluated over the
pooled rows (Margin = total Profit / total Revenue) and a stored measure folds
its records under its own aggregator. The user ruled on three cases:
- **Text** follows the unanimity rule (gotcha 7): the text if every row agrees,
otherwise the `mixed` error.
- **An error** in any row makes the total an error, which is what that rule
answers for an error among numbers. That holds for a **stored** error. A
formula column is evaluated over the pooled inputs, so a row's `div/0` is not
an input and does not propagate. Only the fold's own error does. Rulings (1)
and (2) disagree here: the column shows an error in one row and a clean
number under it. The user chose the **pooled ratio** (improvise-b5dp). Margin
over rows 25/30, 27/30 and -4/0 totals 48/60 = 0.80, not `div/0`.
- **With `_Measure` on the row axis** the strip is empty, because a column adds
different measures together. An empty strip already means "no totals"
(gotcha 33).
The coverage is the **drawn** rows, as a fold's is (improvise-f2oo): a hidden
row stays out, and so does a record that lacks a row coordinate. That
deliberately differs from `Axis::None`, which counts both. The browser and the
TUI agree by construction, since both draw the projected `GridCache.totals`.
The cost is one more evaluation per column, and it is not free. The same
`cargo bench --bench grid_projection` on the orchestrator's Mac (Apple M4 Max),
before and after:
| case | whole grid before | after | `col_widths` before | after |
|---|---|---|---|---|
| typical (400) | 0.86 ms | 1.24 ms | 0.53 ms | 0.92 ms |
| 18k | 114 ms | 177 ms | 75 ms | 142 ms |
| pathological (90,000) | 1.54 s | 3.49 s | 1.07 s | 1.96 s |
`col_widths` is the keypress path, and it pays too, because a total's text sizes
its column. The event loop draws once per key, so every key pays the fold twice:
once in `App::cmd_context` for `visible_cols`, and once in the draw's `grid_of`.
At 4.8k cells that took a held arrow key from about 31 ms to about 52 ms, past
the ~33 ms key-repeat interval (improvise-yb4b). The walk no longer carries a
sum, so the window-into-totals fault below cannot happen any more. The test
still guards the widths half.
**The fold scans instead of enumerating (improvise-yb4b).** The cost was not the
fold's arithmetic. It was *how the fold found its records*: `fold_stored` built
one `CellKey` per member, the cross product of every spanned category's covered
items (3,000 keys for an 18k column), and looked each one up. Now
`group_expansion` returns the expansion as data: a `base` key and the covered
items of each expanded category. `fold_stored` hands that to
`DataStore::spanned_values`, which narrows through the index and filters each
candidate by membership. In `Exact` mode a candidate must also carry no
coordinate beyond those, which is what `get(member)` answered. Only a formula
still enumerates members (`contexts_of`), since its references are evaluated
per context.
**How it narrows is a choice, and the first version got it wrong
(improvise-jq0m).** Narrowing by `base` alone is right for a column total, which
spans *every* item of each row category. It is wrong for a collapsed group,
which spans two or seven items of a category with hundreds: `base` drops the
grouped category entirely, so every fold cell scanned every record of the
column, and a grid of small groups went quadratic. The review measured 1000
days in pairs at 11–15.7 s a keypress, against 0.44–0.51 s before #239. So
`spanned_positions` now prices both narrowings off the index's own set sizes
and takes the cheaper: once by `base`, or once per covered item of the span
with the fewest items, each intersected with `base`. The per-item positions are
disjoint (a record has one item per category) and are sorted afterwards, so
both paths return the store's order. A `base` naming a name the store never
interned matches nothing on both paths.
The bench now carries the grouped cases, which it could not see before: Day in
collapsed groups on Row, with Product, and `_Measure` on Column. The machine is
an Apple M4 Max (16 cores, 64 GB) at load average 15–63. The figures are
keypress (`whole-grid + col_widths`), min-of-N, from two alternating rounds,
the second reversed. Each cell gives both rounds:
| case | before #239 (`5932ad74`) | `f668ddc9` | jq0m | jq0m ÷ before |
|---|---|---|---|---|
| 4.8k dense | 33.9 / 39.1 ms | 40.7 / 40.2 ms | 36.3 / 33.3 ms | 1.07 / 0.85 |
| 18k dense | 237 / 239 ms | 238 / 232 ms | 196 / 211 ms | 0.83 / 0.88 |
| 90k dense | 3.04 / 3.46 s | 3.24 / 2.70 s | 2.68 / 2.84 s | 0.88 / 0.82 |
| 364 days in weeks | 483 / 415 ms | 1.42 / 1.18 s | 356 / 399 ms | 0.74 / 0.96 |
| 364 days in pairs | 715 / 539 ms | 5.02 / 4.62 s | 471 / 518 ms | 0.66 / 0.96 |
| 1000 days in pairs | 406 / 399 ms | 10.8 / 20.2 s | 352 / 364 ms | 0.87 / 0.91 |
The grouped cases now come in **below** the pre-#239 figures. The old
per-member path looked up each member through a full `CellKey`, and the
per-item narrowing intersects index sets directly.
Two levers were **not** taken. Combining the walk's row values for decomposable
aggregators was rejected under §5 as a second derivation of the total. It is
also wrong wherever a formula owns a row-axis item: the row reads the formula,
while the spanning fold reads the store. Sharing one total between
`cmd_context` and the draw was also rejected: a key's effects run between the
two, so sharing needs an invalidation story. Dropping the totals from the widths
would make the two width computations disagree about `visible_cols`.
**A grid-wide fold memo (improvise-h6z1)** is the other half, and it answers a
different cost. It removes the repeated **formula** chains that a fold row and a
total re-derive, about 2x on a folded five-formula chain. It does **not** move
the table below. Those grids hold only stored measures, a stored fold is never
memoised, and each total reads its column's records once. That one read per
column is what the scan above made cheap.
**What it changes besides speed:** a stored fold's values now arrive in the
store's insertion order, not member order. That is the order gotcha 18 names as
the store's answer. Min, Max, Count and text unanimity are order-free. A Sum or
Avg over non-integral values may differ in the last bits.
Measured by alternating three builds of `cargo bench --bench grid_projection` on
an Apple M4 Max (16 cores, 64 GB) under heavy shared load (load average 30–90).
This is min-of-N, three rounds, with the order reversed in the third. The
keypress figure is `whole-grid + col_widths`. The millisecond columns are
ranges over the rounds. The ratio columns are each round's own pair, in round
order:
| cells | before #239 (`5932ad74`) | #239 (`9948aaf5`) | yb4b | #239 ÷ before | yb4b ÷ before |
|---|---|---|---|---|---|
| 400 | 1.36–1.77 ms | 2.36–2.76 ms | 1.69–1.82 ms | 2.03, 1.34, 1.78 | 1.24, 0.98, 1.25 |
| 1.2k | 4.1–5.2 ms | 7.2–8.2 ms | 5.0–5.3 ms | 2.00, 1.39, 1.85 | 1.23, 0.96, 1.25 |
| 2.4k | 10.8–13.6 ms | 18.4–21.5 ms | 12.0–13.9 ms | 1.98, 1.36, 1.91 | 1.18, 0.88, 1.29 |
| 4.8k | 32.8–41.8 ms | 55.4–64.1 ms | 34.5–42.0 ms | 1.77, 1.32, 1.92 | 1.15, 0.82, 1.26 |
| 9.6k | 102.8–121.2 ms | 190.7–209.6 ms | 107.2–140.0 ms | 2.04, 1.57, 1.92 | 1.36, 0.88, 1.20 |
| 18k | 192.9–208.3 ms | 338.6–448.0 ms | 230.4–273.3 ms | 2.32, 1.63, 1.72 | 1.42, 1.11, 1.23 |
| 90k | 2.68–3.26 s | 4.92–6.49 s | 3.18–4.00 s | 1.99, 1.84, 1.79 | 1.23, 1.47, 1.14 |
Read the ratio columns, not the milliseconds. Round 2's `before` ran at load
86 against about 31 for its `yb4b`, which is why its ratios fall below 1. The
two clean rounds put #239 at about 1.8–2.3× the pre-#239 keypress, and put
yb4b at about 1.15–1.4×. At 4.8k a held key is back under the ~33 ms repeat
interval in the quietest round (34.5 ms against 32.8 ms before).
The guard is `a_window_narrows_the_cells_and_neither_the_totals_nor_the_widths`:
with one walk serving three consumers, a window that leaked into the other two
would silently make a column total the sum of the visible rows, or size a column
@@ -3417,12 +3602,17 @@ the one writer that must see the synthetic key: renaming a coordinate column,
creating a column and staging a drill edit all resolve the records position
inside `commit_cell_value`. Its door refuses the two arms `i` refuses, an
aggregate (a fold included) and a formula's result. It needed one because
`Enter` and `Tab` advance the editor, and `EnterEditAtCursor` re-opens it at the
`Enter` and `Tab` advance the editor, and `EnterEditAtCursor` re-opened it at the
new cursor with no guard. `i` declines to edit `Profit`, yet `Tab` from `Cost`
landed the editor on it, and the next commit stored a value under the formula.
That is a fourth door nobody had listed. **The entry half is still open**
(improvise-enz6): the editor still opens on a fold or a formula's cell. Only the
write is refused, out loud, at commit. Closing the entry belongs in `ui/effect.rs`.
That is a fourth door nobody had listed. **The entry half is closed too**
(improvise-enz6). The door's two arms are now one function,
`core::uneditable`, and `EnterEditAtCursor` asks it before opening. So the
re-opened editor and the commit that would follow it cannot disagree about which
cells they decline. The commit door stays: a `:commit-cell-edit` line runs it at
the cursor with no editor opened there. Since the advance no longer reaches it,
the door has tests of its own that seat the editor directly
(`an_editor_already_open_on_a_fold_commits_nothing_there` and its formula twin).
**Drill keeps the narrowest door** because drilling into an aggregate is its
purpose. Only a fold is refused, since no record carries a group's name. The
@@ -3607,10 +3797,32 @@ the refusal marks the batch, `Host::mutating_frame` undoes the frame, the asker
gets `Rejected(MutationRefused)`, and the reason is on the host's status line.
The question it asks is the projection's own, never a second derivation of it:
- A coordinate that names a **group** rather than an item is refused, in any
view (improvise-b114). A group has no cell of its own, so writing one stores a
non-item coordinate. This rule asks the document, not the layout, and runs
first, because a fold in the measure half hides from both rules below. Take
- A coordinate that names **no item of its category** is refused, in any view
(improvise-b114, then improvise-nacd). This rule asks the document, not the
layout, and runs first, because a fold in the measure half hides from both
rules below.
**It is stated positively, and that is the point of the nacd change.** The
b114 rule was negative: refuse a name that *is a group*. The invariant it
protects (improvise-gw18) is positive: every coordinate names an item. A
negative rule covers only the ways of not being an item that someone listed,
and review found two more within a day. `Bogus` is no item and no group. A
parent group such as `Totals`, above `Money`, holds no item directly, so
`items_in_group("Totals")` is empty and the group test missed it. Both wrote
`{Month=Feb, _Measure=…}` onto a stored record and were accepted. The
positive rule covers both, and any shape not yet thought of, because it asks
whether the name *is* an item rather than what else it might be.
"Item" means `Model::effective_item_names`: the category's items, plus the
formula targets for `_Measure`. The formula targets are included so that a
write under a formula passes this rule and is refused by the next one with
the reason that is actually true ("computed by a formula"), not as a non-item.
The same-name case falls out: an item called `Money` beside a group called
`Money` is an item, and is writable (improvise-pr8t). The b114 rule needed an
explicit exemption for that case, and nothing tested the exemption.
A group has no cell of its own, so writing one stores a non-item
coordinate. Take
`{Month=Feb, _Measure=Money}`, where `Money` groups Revenue and Cost. Its
record `{Month=Feb}` is stored, so the second rule below accepts it and never
asks the projection. And with `Money` expanded the host draws no such cell at
context/code/core.md
+21 -8
View File
@@ -929,16 +929,29 @@ every member. Five decisions:
without the memo, about 60 ms with it, and about 10 ms on gh/main. gh/main
was cheap because it read the members out of the formula cache, which is
what gave the wrong ratios. The remaining ~6× is each column's cell
re-deriving the chain below it (45 evaluations per row where 9 would do). A
memo scoped to the whole grid would remove it; it is not built.
re-deriving the chain below it (45 evaluations per row where 9 would do).
**The memo is now shared across one grid build** (improvise-h6z1).
`viewport::grid_body` makes one `FoldMemo` and passes it through
`GridLayout::evaluate_in` and `column_total` for every cell and total, so a
fold row's chain is evaluated 9 times, not 45. A memo is valid only for the
model state it read. That is why it is made per build and never stored on
`GridLayout`, which outlives cell writes.
The memo is keyed by the fold key **and** the span's categories. Within one
grid build the span half cannot yet differ for one key, because a cell's key
always names the row categories and a total's never does. It is kept because
`evaluate_spanning_in` is public, and a caller sharing one memo across two
spans would otherwise read the wrong fold. No test can see it today.
- **A fold covers the members the grid draws** (improvise-f2oo). Hidden members
are not in it. So a fold is exactly the rows it replaces, the total of a
**stored** measure under Sum or Count does not move on `z`, and a group whose
every member is hidden has no row. Under Avg, Min or Max a stored measure's
total does move, and so does a **formula** column's (PR #203's review measured
Margin 1.90 expanded, 1.30 collapsed). The strip sums the rows' values rather
than folding the column the way a fold does, so it sums a Max row's maximum
and a formula row's result. That is improvise-wm20, and it is not this rule. That last
column's total does not move on `z`, and a group whose every member is hidden
has no row. That holds for every aggregator and for a formula column only
since improvise-wm20, because the strip used to **sum** the drawn rows. That
moved a Max measure's total on `z`, and a ratio's (PR #203's review measured
Margin 1.90 expanded and 1.30 collapsed). A total is now the column's cell as
if every drawn row were one fold, so collapsing a group changes nothing it
reads. The fold's machinery is shared through `Model::evaluate_spanning`: its
`Span` adds, to the groups a key names, the categories a key **omits**, which
it expands to their covered items. That last
point was already true, and it contradicted the old rule, under which a hidden
value appeared only while a sibling was visible. This deliberately **differs
from `Axis::None`**, where hidden items still count, because the two replace
context/code/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
+261 -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
@@ -4534,6 +4542,8 @@ Two faults injected:
assertion (`left: Some("0.50") right: Some("0.5000")`).
- Dropping only the `col_formats` write in `viewport::grid_body`, leaving the
cells correct, fails the totals assertion (`left: "1.50" right: "1.5000"`).
Those numbers predate improvise-wm20. The total is now the formula over the
pooled rows, `0.5000`, and the same rival reads `0.50`.
The second is the one that earns the test its length: without it, the totals
strip could round every formula column to the view's format and no assertion in
@@ -5344,11 +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
@@ -5380,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
@@ -5919,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.
@@ -5941,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
@@ -6156,6 +6293,45 @@ flag. This test fails alone. Every other test stays green, which was the gap.
Once improvise-fy9o re-seeds the reopened editor, a stale `touched` would make
every untouched Tab or Enter write its seed.
## `ui/project.rs` — the totals strip (improvise-wm20)
### a_ratio_columns_total_is_the_ratio_of_the_totalled_inputs
improvise-wm20. `Margin = (Revenue - Cost) / Revenue` over North (10, 5) and
South (30, 3). The rows read 0.50 and 0.90, so the three candidate rules give
three different answers: summed **1.40**, averaged 0.70, and pooled
(40 − 8) / 40 = **0.80**. A fixture whose rows share a ratio cannot tell them
apart; the old `a_formulas_format_reaches_its_own_column_and_no_other` is one
such fixture. **Red at `60179150`** with 1.40. Rival: the red commit itself,
where the walk sums.
### an_average_measures_total_is_the_mean_over_its_records
improvise-wm20. `Price` under Avg, with `Product` on None, so North's row is the
mean of two records (10, 20) and South's of one (30). Summing the rows gives
**45**, the mean of the rows' means 22.5, and the mean of the records **20**.
The ruling is the last. **Red at `60179150`** with 45.
### a_hidden_rows_values_stay_out_of_the_total
improvise-wm20. `East` (100, 100) is hidden, so both the Revenue total (40) and
the pooled Margin (0.80) must leave it out. Pooled in, the Margin would read
32/140. This test earns its place through two rivals, each injected into a full
workspace run where each fails this test and nothing else:
- The span ignores `covers`, so hidden rows are pooled in.
- A formula's references are read **unspanned**. The ratio test cannot see this
one: an unspanned reference reads the column key over every record, which is
the right answer whenever nothing is hidden. Only a hidden row separates
"every drawn row" from "every record".
### a_max_measures_total_does_not_move_when_a_row_group_collapses
improvise-wm20, from the #203 re-review's comment on the bead. `Revenue` under
Max, with Region on None and Month's Jan and Feb in Q1. Expanded, the rows are
5 and 7, which a summing strip reads as **12**. Collapsed, the fold reads 7. The
total must be 7 both ways. **Red at `60179150`** with 12.
#### improvise-i3o5: four declines that `set_status` hid from every sweep
`every_refusal_site_refuses_its_failure_and_only_its_failure` gained four cases.
@@ -6185,6 +6361,29 @@ runs `cargo test --workspace --no-fail-fast`. Four of four fail this test, and
nothing else fails. None of the four needs an abort: each is the only effect its
command emits, and `p`/`i` are single bindings, not sequences.
### a_formula_rows_error_does_not_poison_the_pooled_total
improvise-b5dp, the user's ruling on the conflict between wm20's rulings (1) and
(2): a formula column's total is the **pooled ratio**. A row's formula error
does not poison it, and a stored error still does
(`an_error_in_any_row_makes_the_total_an_error`). Rows North 25/30, South 27/30
and East -4/0 draw 0.83, 0.90 and `div/0`. The total must read **0.80**
(48/60), not an error. The test asserts that East's row really draws the error,
so a fixture where no row errors cannot pass it.
It was **green at `9948aaf5`**, because #239 already implemented the choice the
user then ratified. It is a pin, not a reproduction. Rival: in `grid_body`'s
walk, a drawn cell's `Error` overwrote its column's total. That is ruling (2)
applied to every row. This test and its Share sibling fail on the total
assertion. The other seven `column_totals` tests pass.
### a_share_columns_total_pools_past_a_rows_div_zero
improvise-b5dp, the second case the bead named. `Share = Revenue /
SUM(Revenue)`: East's own `SUM` is 0, so its row draws `div/0`. The pooled
total is 60/60 = **1.00**. It was green at `9948aaf5` and fails under the same
rival as the test above.
### a_frame_that_reframes_and_then_places_the_cursor_leaves_the_asker_placed
improvise-wdmp. A browser frame that reframes the grid and then places the cursor,
@@ -6239,3 +6438,57 @@ behavioural one is the reason the rebuild exists.
Placed in `ui/host.rs` rather than in `ui/app.rs`'s `snapshot_restore` module
(which PR #254 adds), both to avoid that conflict and because the consequence is
the host's.
## `ui/host.rs` — the abort arm of `Host::mutating_frame` (improvise-we1a)
### every_failing_model_effect_that_aborts_also_refuses
What the gap was: `Host::mutating_frame` rejects a frame when
`last_batch_refused() || last_batch_aborted()`. Every model effect that aborted
also refused, so deleting the `|| last_batch_aborted()` arm left all tests
passing. The wire table (`every_failing_model_command_refuses_its_whole_frame`)
cannot hold the invariant, because that arm rejects a frame that aborted
without refusing. The test applies each failing `ModelCommand`'s own effects,
from `model_effects`, straight to an `App`, and asserts that the batch refused.
Each case also carries whether it aborts. That catches a table in which nothing
aborts, and a site that quietly stops aborting.
Faults injected:
- `AddCategory`'s failure arm set `status_msg` and aborted, but did not refuse.
- This test failed, and so did the key-path table.
- The wire table passed, which is the gap.
- With the arm also deleted, `a_refused_mutation_rolls_back_its_predecessor_and_the_abort_flag`
failed as well. So the arm is what makes the wire table look correct under
this fault.
- `refuse_and_stop` wrote the status and aborted without refusing. This test
failed, as did the key-path table,
`a_failed_load_or_import_does_not_run_the_quit_after_it` and
`a_formula_that_does_not_parse_keeps_the_user_in_the_editor_with_their_text`.
The wire table passed.
What this table cannot see (improvise-lasd, found in review of #229): a site it
does not list. The fault injected was `on_view`'s missing-view branch changed to
write the status and abort, without refusing. The workspace stayed green, and
only `Host::mutating_frame`'s `|| aborted` arm caught it. That is why
`App::apply_effects` now carries a `debug_assert!`: a `Model` effect that aborts
must have refused. Under the same fault, the assertion fails two existing tests:
- `a_frame_writing_a_view_its_own_first_command_deleted_is_refused`, with
`SetNumberFormat(Named("Scratch"), ",.2f")`;
- `a_command_naming_a_view_the_document_has_not_got_writes_nothing`, with
`SetAxis { view: Named("NoSuchView"), .. }`.
With the assertion removed, the same fault fails nothing.
### every_refusal_site_refuses_its_failure_and_only_its_failure — three cases (improvise-rtud)
This entry covers the rows `load of an absent file`, `import-json of an absent
JSON` and `a formula that does not parse`. Each has a succeeding twin
(`load doc.improv` from `MemoryFs`, `import-json good.json`, `Total = 1`).
Each row was checked by a rival at its own site: the `refuse_and_stop` call in
`AddFormula`'s parse arm, in `LoadModel`, or in the JSON read arm of
`ImportJsonHeadless`, replaced by a status write plus the abort. Under each
rival, the table fails at that row. The matching row of
`every_failing_model_effect_that_aborts_also_refuses` fails too. A rival in
`refuse_and_stop` itself is no use here, because the table stops at its first
failing row, which is the older `import-json of an absent CSV`. The rtud prose correction is landing separately, so these
rows are coverage only.
context/test-evidence/core.md
+67
View File
@@ -1511,3 +1511,70 @@ are each compared with the same document with Month on `Axis::None`. At
`Flag` at `Some(0.0)`, false rather than absent, so a fix that dropped the cell
cannot pass. Rival: `read_coordinate` ignoring the reach (`if false && …`).
This test fails.
### a_fold_row_evaluates_each_formula_once_across_the_row
improvise-h6z1. `F0 = Revenue`, `F{i} = F{i-1} + F{i-1}` up to F8, as columns,
with Q1 collapsed so the Q1 row is a fold. The test evaluates every cell of that
row twice and compares the two runs: once with a fresh memo per cell (`evaluate`),
where each column re-derives the chain below it (1 + 2 + … + 9 = **45**), and
once with one memo shared across the row (`evaluate_in`), which gives **9**. It
also asserts the two runs produce identical values, so a memo that returned a
wrong entry cannot pass. The 45 is asserted as a setup, so a fixture whose chain
had collapsed into one column could not pass the 9 by accident. Rival:
`evaluate_covering_in` ignoring the memo it is given and making a fresh one.
Injected into a full workspace run, it fails this test (45 against 9) and
nothing else.
### a_spanned_scan_finds_what_a_lookup_per_member_finds
improvise-yb4b. `fold_stored` used to enumerate one `CellKey` per member of a
fold and look each one up. Since #239 a column total spans every row
category, which is 3,000 lookups for one 18k column, paid twice per keypress.
`DataStore::spanned_values` now answers with one index-narrowed scan. This test
compares the scan with the per-member definition it replaces, in both modes, on
a store built to separate them. It has a record with an **extra coordinate**:
`get` misses it and `matching_values` finds it. It has an **uncovered item**, a
record **lacking** the spanned category, **another measure**, and another
value of an unspanned coordinate. It also has a covered item that was never
interned. The values are distinct powers of two, so a sum identifies the
admitted set. The order differs on purpose: the store's order, not member
order, as gotcha 18 names it.
Three faults were injected, each run against the full `improvise-core` and
`improvise-command` suites:
- Exact mode loses its length check (`==` → `>=`). **Only this test fails.**
Nothing else in either suite reaches an `Exact` fold over a record with an
extra coordinate.
- The span ignores coverage (`|| true`). This test fails, and so do five others
(collapsed-group, hidden-row and CSV-agreement tests).
- A category the store never interned is skipped instead of matching nothing.
**Only this test fails.**
### a_span_finds_what_its_members_find_in_store_order
improvise-jq0m. `spanned_values` now chooses between two narrowings, and a
fixed example can only ever take one of them. This property builds random
stores over `R`, `P` and `M`. Some records lack `R`, some carry an extra `X`,
and `r6` is a covered item the store never interned. It spans `R` and sometimes
`P`, in both modes, and compares the result with the per-member definition,
**in store order**: the values the members' own lookups find, read back in
`iter_cells` order. Store and span sizes vary across cases, so both narrowings
are taken.
Three faults were injected, each run against the full `improvise-core` and
`improvise-command` suites:
- The per-item narrowing drops the base, so it keeps each item's pair alone.
This test and `a_spanned_scan_finds_what_a_lookup_per_member_finds` fail.
**Nothing in either crate's grid or command tests does.** Those fixtures never
hold a second value of the base's coordinate within a small span.
- The base narrowing drops the base. This test fails, along with eight
fold and totals tests.
- The per-item positions are not sorted after they are merged. **Only this
test fails.** Values stay correct and only their order moves, which changes
a Sum or Avg in the last bits.
The regression itself is a cost, which no test here can see. The evidence for
it is the `grid_projection` bench's grouped cases, added for this ticket.
context/test-evidence/formula-and-io.md
+35
View File
@@ -1197,6 +1197,41 @@ a further 0.39s → 0.21s but it is not the fix, and reporting it as one would
be the mistake this entry exists to record. Net: 1.42s → 0.21s, and
`improvise-io` is no longer the run's longest binary.
### parser::generator::the_grammar_is_parsed_once_however_many_formulas_the_strategies_yield
improvise-i5z6 is improvise-8shu's twin, in `improvise-formula`. Both
generator strategies, `formula_string()` and `expr_string()`, called
`load_grammar()` inside their `prop_map`. So `pest_meta` re-parsed
`formula.pest` for every generated case: three 256-case properties plus
shrinks. The `improvise-io` fix never reached this crate, which has its own
copy of the generator.
All figures were measured on an Apple M4 Max (16 cores) under heavy parallel
load (load average 15–33), because an unloaded window was not available. The
ratios are more trustworthy than the seconds.
- **Before:** the lib binary was the **longest binary in all three workspace
runs**, at 0.91s, 1.02s and 1.95s. The runner-up was 0.50–0.92s, and a fourth
run gave 3.13s. The median binary reports `0.00s` at two decimals, so the
ratio to the median is not defined. By any finite reading it is two orders
of magnitude above the median. Alone, the binary took 0.84–0.98s over five
runs. The three generator properties alone took 0.94–1.03s, and the other 82
tests together took 0.01s.
- **After:** the binary alone takes 0.06–0.07s over five runs, and
0.06–0.09s inside the workspace run. `headless_cli`, which spawns processes,
is now the longest binary at 0.49–0.75s.
The guard copies 8shu's shape. It counts **parses**, not calls, so it does not
depend on test order. It drives both strategies through a `TestRunner` rather
than re-implementing them. Red before the fix with `left: 64, right: 1`.
Rivals, one per strategy: restoring `let rules = load_grammar();` in either
`formula_string` or `expr_string` alone fails the test with `left: 33`. So the
two strategies are guarded independently.
The case count stays 256. It is already the default that
`context/design-principles.md` §7 prescribes, and it was never the cost.
### export_csv_omits_the_rows_the_view_prunes
improvise-iot6. `render_csv` built its layout with `GridLayout::new`, which does
context/test-evidence/tooling.md
+231
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,105 @@ So the pass needs both the exclusions and git's file list.
After each rival the original was restored (`cmp` against a saved copy) and the
suite re-run at 8 passed.
### The audit tells grep's no-match from its read error — `scripts/bd-audit-ids-test.sh` (improvise-lqs0)
**What broke.** `bd-audit-ids` treated xargs exit 123 as "a grep batch matched
nothing". xargs also exits 123 when grep exits 2, so a cited file the scan could
not read passed the audit, and any dead id inside it was missed. The usage text
promised exit 4 for that. Two smaller mismatches with the stated contract:
- run outside a git repository, the audit exited 1, which is the "dead id" code;
- given a bd list whose objects carry no `id`, it reported every cited id dead
(exit 1) where "bd did not answer" is 3.
**Reproduced before the fix, with six new cases:** 10 passed, 4 failed.
- a dangling symlink among the cited files: exit 1, the read error ignored;
- an unreadable (`chmod 000`) file holding the only dead id: **exit 0, "all
resolve"**, which is the false pass;
- outside a repository: exit 1;
- id-less JSON: exit 1.
The control (both files readable, dead id found: exit 1) and the deleted-file
case passed before and after.
**The fix:**
- Each grep batch runs through `bash -c` and maps exit 1 to success and exit 2
to 255. xargs stops on 255 with 124, so a read error is exit 4.
- A tracked file deleted from the working tree is filtered out **on purpose**.
Its ids are leaving the tree, which is the case the ticket asked to decide
rather than leave to accident. A dangling symlink is still an entry, so it
still counts as a read error.
- Outside a repository is exit 4.
- bd's output must be a JSON array whose every element has a string `id`, or
the audit is exit 3.
**Rivals, each run against the suite (the script restored and checked with
`cmp` after each):**
- **read error mapped to success** (`-le 2`): fails both read-error cases, 12
passed and 2 failed;
- **the deleted-file filter removed**: fails "a tracked file deleted from the
working tree is not scanned" with exit 4. So the decision is pinned, not
incidental;
- **the old `cd "$(git rev-parse --show-toplevel)"`**: fails "run outside a git
repository is exit 4" with exit 1. A first attempt at this rival still exited
4 and proved nothing; it was replaced;
- **the id check reduced to `type == "array"`**: fails the id-less case with
exit 1.
The `chmod 000` case is skipped when the suite runs as root, where the file
stays readable. The dangling-symlink case covers the same path for every user.
### Absent is not deleted — `scripts/bd-audit-ids-test.sh` (improvise-tdfd)
**What broke.** The file filter `[ -e "$f" ] || [ -L "$f" ]` dropped every
tracked path missing from the tree, and took each one to be a deletion. A path
marked `--skip-worktree` (or left out by a sparse checkout) is absent too, but
it is not deleted. Its ids were never read, and the audit passed. ChatGPT
Codex found this on PR #244. The adversarial review reproduced it with
`context/adr/001-concurrent-edits.md`, the only citation of one id: exit 1 → 0.
**Fix, Codex's suggestion.** An absent path is skipped only when
`git ls-files -d` lists it. Any other absent tracked path is exit 4, and the
path is named. The xargs comment no longer claims 124. That is GNU; BSD exits
1. The script tests nonzero, so both work.
**Red first.** "a skip-worktree file missing from the tree is exit 4, and
named" exited 0 against the pre-fix script, reporting "all 1 cited ids
resolve".
**Rival:** every absent path reported as unread (the `ls-files -d` test
removed). It fails "a tracked file deleted from the working tree is not
scanned" alone, so the deleted-file case is what stops the fix from
over-refusing. The rival ran from an ignored copy inside the worktree. A copy
under `/tmp` fails "the real tree does not pass against a silent bd" for a
reason unrelated to the rival: there is no repository there.
### Section 5b runs on a hermetic PATH — `scripts/checkers-test.sh`
**What broke.** Every PR's "Knowledge base consistency" job went red at
`666de199` with "a bash 5.0 with no nix is exit 3, not 5.x (exit 1, wanted 3)"
and "a bash 5.1 is accepted (exit 1, wanted 0)". Neither PR alone was wrong. The
NixOS-runner change (`af44653f`) found that `/usr/bin:/bin` on NixOS holds only
`env` and `sh`, so the runner dies on `dirname` with exit 1. It fixed section 5
by moving it to a `utilbin` of linked tools. #237 (`e45a9c42`) added section 5b
on its own branch, with the old `PATH=$work/fakebin:/usr/bin:/bin`. The two
merged with no conflict, and each had passed CI only on its own side.
**Reproduced** on macOS by running both 5b invocations with `/usr/bin:/bin`
replaced by a directory holding only `env` and `sh`. Both exited 1 on
`dirname: command not found`. Those are the CI exits.
**Fix.** 5b uses `utilbin` as well. `utilbin` gains `head` and `env`, because
the 5.1 control runs past the version check into `check_shebangs` and
`run_suite`.
**Rivals**, each run in the suite:
- **`dirname` left out of `utilbin`** fails all three section-5 cases with the
exact CI pair: exit 1 where 3 was wanted, and exit 1 where 0 was wanted. The
NixOS failure now reproduces on any host, because no case reads the host's
`/usr/bin` any more.
- **`env` left out** fails "a bash 5.1 is accepted" alone.
- **`head` left out** fails nothing. `check_shebangs` calls `head` inside
`check_shebangs || rc=1`, where `set -e` is off. A missing `head` makes every
first line empty, and the shebang check then passes without checking
anything. That is a defect in `checkers.sh` and is not fixed here.
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
@@ -1533,6 +1533,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() {
@@ -1704,6 +1718,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]
@@ -2451,6 +2518,112 @@ mod tests {
}
}
/// improvise-we1a: `mutating_frame`'s `|| last_batch_aborted()` arm is redundant
/// only while this holds. Evidence: `context/test-evidence/command.md`.
#[test]
fn every_failing_model_effect_that_aborts_also_refuses() {
let fresh = || {
let mut wb = workbook();
wb.create_view("Scratch");
(0..12).for_each(|n| drop(wb.add_category(format!("Filler{n}"))));
let mut app = App::new(wb, None);
app.storage = Box::new(super::import_fixture_storage());
app.rebuild_layout();
app
};
let cases: Vec<(&str, ModelCommand, bool)> = vec![
(
"AddCategory past the limit",
ModelCommand::AddCategory("Extra".into()),
true,
),
(
"AddFormula that does not parse",
ModelCommand::AddFormula {
raw: "Total = (".into(),
target_category: "_Measure".into(),
},
true,
),
(
"LoadModel of an absent file",
ModelCommand::LoadModel(PathBuf::from("absent.improv")),
true,
),
(
"import of an absent JSON",
import("absent.json", None),
true,
),
("import of an absent CSV", import("absent.csv", None), true),
(
"import of a CSV that is not text",
import("bad.csv", None),
true,
),
(
"import of JSON that does not parse",
import("bad.json", None),
true,
),
(
"import at a path with no array",
import("nested.json", Some("nope")),
true,
),
(
"import that builds no model",
import("measures.json", None),
true,
),
(
"AddItem into an absent category",
ModelCommand::AddItem {
category: "Ghost".into(),
item: "East".into(),
},
false,
),
(
"AddItemInGroup into an absent category",
ModelCommand::AddItemInGroup {
category: "Ghost".into(),
item: "East".into(),
group: "Coastal".into(),
},
false,
),
(
"AddFormula into an absent category",
ModelCommand::AddFormula {
raw: "Total = 1".into(),
target_category: "Ghost".into(),
},
false,
),
(
"DeleteView of an absent view",
ModelCommand::DeleteView("Ghost".into()),
false,
),
];
for (what, cmd, aborts) in cases {
let mut app = fresh();
let effects = crate::ui::reduce::model_effects(&app, &cmd);
app.apply_effects(effects);
assert_eq!(
app.last_batch_aborted(),
aborts,
"{what}: which failures abort is part of what this table pins"
);
assert!(
app.last_batch_refused(),
"{what}: failed without refusing, so a frame carrying it is rejected \
only by mutating_frame's `|| last_batch_aborted()` arm"
);
}
}
/// improvise-9fpl: one case per refusal #181 converted that no other test
/// witnesses. Evidence: `context/test-evidence/command.md`.
#[test]
crates/improvise-command/src/ui/project.rs
+274 -4
View File
@@ -256,11 +256,22 @@ fn column_totals(body: &GridBody, cols: std::ops::Range<usize>) -> Vec<CellDispl
true => Vec::new(),
false => cols
.filter_map(|c| Some((body.totals.get(c)?, body.col_formats.get(c)?)))
.map(|(total, fmt)| CellDisplay::number(fmt.number(*total)))
.map(|(total, fmt)| total_display(total.as_ref(), fmt))
.collect(),
}
}
/// One total as the strip draws it: a number, a unanimous text, an error, or
/// nothing where the column holds no record (improvise-wm20).
fn total_display(total: Option<&CellValue>, fmt: &crate::format::NumberFormat) -> CellDisplay {
match total {
Some(CellValue::Number(n)) => CellDisplay::number(fmt.number(*n)),
Some(v @ CellValue::Error(_)) => CellDisplay::error(fmt.value(Some(v))),
Some(v @ CellValue::Text(_)) => CellDisplay::text(fmt.value(Some(v))),
None => CellDisplay::default(),
}
}
/// One cell as the client draws it. The server knows whether a value was a number
/// and whether the cell is settable, so it says both (ADR 002, [`cell_id`]).
fn cell_display(
@@ -2951,7 +2962,7 @@ mod tests {
);
assert_eq!(
one_row.totals,
vec![1.0],
vec![Some(CellValue::Number(1.0))],
"a column total sums every row, including the ones scrolled past"
);
assert_eq!(
@@ -4854,8 +4865,8 @@ mod cell_identity {
// The totals strip is formatted on its own path (`col_formats`), so it
// can disagree with the cells above it without any cell being wrong.
assert_eq!(
grid.totals[margin].text, "1.5000",
"three rows of 0.5, under the column's own format"
grid.totals[margin].text, "0.5000",
"the formula over the pooled rows (3 / 6), under the column's own format"
);
assert_eq!(grid.totals[revenue].text, "3.00");
}
@@ -5820,3 +5831,262 @@ mod basis_check {
assert_eq!(value_at(&app, 1), "99.00");
}
}
/// The totals strip: a column's total is its cell as if every drawn row were one
/// fold (improvise-wm20). The rule and the rulings: `context/code/command.md`.
#[cfg(test)]
mod column_totals {
use super::*;
use crate::model::cell::{CellKey, CellValue};
use crate::workbook::Workbook;
use improvise_core::view::Axis;
const WHOLE: Viewport = Viewport {
row_offset: 0,
col_offset: 0,
rows: 20,
cols: 20,
};
fn key(coords: &[(&str, &str)]) -> CellKey {
CellKey::new(
coords
.iter()
.map(|(c, i)| (c.to_string(), i.to_string()))
.collect(),
)
}
/// Region on Row, `_Measure` on Column, one record per cell.
fn regions(cells: &[(&str, &str, CellValue)]) -> Workbook {
let mut wb = Workbook::new("T");
wb.add_category("Region").unwrap();
for (region, measure, value) in cells {
wb.add_item("Region", *region).unwrap();
wb.add_item("_Measure", *measure).unwrap();
let at = key(&[("Region", region), ("_Measure", measure)]);
wb.model.set_cell(at, value.clone());
}
wb.active_view_mut().set_axis("Region", Axis::Row);
wb.active_view_mut().set_axis("_Measure", Axis::Column);
wb
}
fn total(wb: Workbook, measure: &str) -> CellDisplay {
let mut app = App::new(wb, None);
app.rebuild_layout();
let col = (0..app.layout.col_count())
.find(|&c| app.layout.col_label(c) == measure)
.unwrap_or_else(|| panic!("no column {measure}"));
grid_of(&app, WHOLE).totals[col].clone()
}
fn money(rows: &[(&str, f64, f64)]) -> Workbook {
let cells: Vec<_> = rows
.iter()
.flat_map(|(r, rev, cost)| {
[
(*r, "Revenue", CellValue::Number(*rev)),
(*r, "Cost", CellValue::Number(*cost)),
]
})
.collect();
regions(&cells)
}
/// improvise-wm20 — see context/test-evidence/command.md.
#[test]
fn a_ratio_columns_total_is_the_ratio_of_the_totalled_inputs() {
let mut wb = money(&[("North", 10.0, 5.0), ("South", 30.0, 3.0)]);
let margin =
crate::formula::parse_formula("Margin = (Revenue - Cost) / Revenue", "_Measure");
wb.model.add_formula(margin.unwrap());
// Rows are 0.50 and 0.90: summed 1.40, averaged 0.70, pooled 32/40.
assert_eq!(total(wb, "Margin").text, "0.80");
}
/// improvise-wm20: the fold is of the DRAWN rows, so a hidden row's values stay
/// out of every total, a formula's included. context/test-evidence/command.md.
#[test]
fn a_hidden_rows_values_stay_out_of_the_total() {
let mut wb = money(&[
("North", 10.0, 5.0),
("South", 30.0, 3.0),
("East", 100.0, 100.0),
]);
let margin =
crate::formula::parse_formula("Margin = (Revenue - Cost) / Revenue", "_Measure");
wb.model.add_formula(margin.unwrap());
wb.active_view_mut().hide_item("Region", "East");
assert_eq!(total(wb.clone(), "Revenue").text, "40.00");
assert_eq!(
total(wb, "Margin").text,
"0.80",
"East's 0 margin pooled in"
);
}
/// improvise-wm20: an Avg measure totals as the mean over its records, not
/// the sum or mean of the rows' means. context/test-evidence/command.md.
#[test]
fn an_average_measures_total_is_the_mean_over_its_records() {
let mut wb = Workbook::new("T");
wb.add_category("Region").unwrap();
wb.add_category("Product").unwrap();
wb.add_item("_Measure", "Price").unwrap();
for (region, product, price) in [
("North", "A", 10.0),
("North", "B", 20.0),
("South", "A", 30.0),
] {
wb.add_item("Region", region).unwrap();
wb.add_item("Product", product).unwrap();
let at = key(&[
("Region", region),
("Product", product),
("_Measure", "Price"),
]);
wb.model.set_cell(at, CellValue::Number(price));
}
let avg = crate::formula::AggFunc::Avg;
wb.model.measure_agg.insert("Price".into(), avg);
wb.active_view_mut().set_axis("Region", Axis::Row);
wb.active_view_mut().set_axis("_Measure", Axis::Column);
wb.active_view_mut().set_axis("Product", Axis::None);
// Rows are 15 and 30: summed 45, their mean 22.5, the records' mean 20.
assert_eq!(total(wb, "Price").text, "20.00");
}
/// improvise-wm20, ruling 1: a text column is unanimous or `mixed`.
#[test]
fn a_text_columns_total_is_its_text_when_every_row_agrees() {
let text = |s: &str| CellValue::Text(s.into());
let agreed = regions(&[
("North", "Status", text("ok")),
("South", "Status", text("ok")),
]);
let t = total(agreed, "Status");
assert_eq!((t.text.as_str(), t.error), ("ok", false));
let split = regions(&[
("North", "Status", text("ok")),
("South", "Status", text("no")),
]);
let t = total(split, "Status");
assert!(t.error, "differing text totals as an error: {t:?}");
}
/// improvise-wm20, ruling 2: an error in any row makes the total an error.
#[test]
fn an_error_in_any_row_makes_the_total_an_error() {
let wb = regions(&[
("North", "Revenue", CellValue::Number(10.0)),
("South", "Revenue", CellValue::Error("bad".into())),
]);
let t = total(wb, "Revenue");
assert!(t.error, "a row's error was dropped from the total: {t:?}");
}
/// improvise-wm20, ruling 3: with `_Measure` on the row axis a column adds
/// different measures together, so there is no strip at all.
#[test]
fn measures_on_the_row_axis_leave_no_totals_strip() {
let mut wb = money(&[("North", 10.0, 5.0), ("South", 30.0, 3.0)]);
wb.active_view_mut().set_axis("Region", Axis::Column);
wb.active_view_mut().set_axis("_Measure", Axis::Row);
let mut app = App::new(wb, None);
app.rebuild_layout();
assert!(grid_of(&app, WHOLE).totals.is_empty());
}
/// improvise-wm20 with #203's folds: a Max measure's total does not move when
/// a row group collapses. context/test-evidence/command.md.
#[test]
fn a_max_measures_total_does_not_move_when_a_row_group_collapses() {
let book = |collapsed: bool| {
let mut wb = Workbook::new("T");
wb.add_category("Month").unwrap();
wb.add_category("Region").unwrap();
wb.add_item("_Measure", "Revenue").unwrap();
for (month, n, s) in [("Jan", 5.0, 1.0), ("Feb", 2.0, 7.0)] {
wb.add_item_in_group("Month", month, "Q1").unwrap();
for (region, v) in [("North", n), ("South", s)] {
wb.add_item("Region", region).unwrap();
let at = key(&[
("Month", month),
("Region", region),
("_Measure", "Revenue"),
]);
wb.model.set_cell(at, CellValue::Number(v));
}
}
let max = crate::formula::AggFunc::Max;
wb.model.measure_agg.insert("Revenue".into(), max);
wb.active_view_mut().set_axis("Month", Axis::Row);
wb.active_view_mut().set_axis("_Measure", Axis::Column);
wb.active_view_mut().set_axis("Region", Axis::None);
if collapsed {
wb.active_view_mut().toggle_group_collapse("Month", "Q1");
}
wb
};
// Expanded rows are 5 and 7, which a strip that sums reads as 12.
assert_eq!(total(book(false), "Revenue").text, "7.00");
assert_eq!(total(book(true), "Revenue").text, "7.00");
}
/// The column's grid and its index, for a test that reads rows and total together.
fn column(wb: Workbook, measure: &str) -> (GridCache, usize) {
let mut app = App::new(wb, None);
app.rebuild_layout();
let col = (0..app.layout.col_count())
.find(|&c| app.layout.col_label(c) == measure)
.unwrap_or_else(|| panic!("no column {measure}"));
(grid_of(&app, WHOLE), col)
}
/// North 25/30, South 27/30, East -4/0: the rows draw 0.83, 0.90, div/0.
fn margins_with_a_zero_revenue_row() -> Workbook {
money(&[
("North", 30.0, 5.0),
("South", 30.0, 3.0),
("East", 0.0, 4.0),
])
}
/// improvise-b5dp, user ruling: a formula total is the pooled ratio, so a row's
/// div/0 does not poison it. context/test-evidence/command.md.
#[test]
fn a_formula_rows_error_does_not_poison_the_pooled_total() {
let mut wb = margins_with_a_zero_revenue_row();
for f in ["Profit = Revenue - Cost", "Margin = Profit / Revenue"] {
let formula = crate::formula::parse_formula(f, "_Measure").unwrap();
wb.model.add_formula(formula);
}
let (grid, col) = column(wb, "Margin");
let rows: Vec<bool> = grid.cells.iter().map(|r| r[col].error).collect();
assert_eq!(rows, vec![false, false, true], "East's row must draw div/0");
let t = &grid.totals[col];
assert_eq!(
(t.text.as_str(), t.error),
("0.80", false),
"48 / 60, pooled"
);
}
/// improvise-b5dp: the same ruling for a formula whose own aggregate is zero
/// at one row. context/test-evidence/command.md.
#[test]
fn a_share_columns_total_pools_past_a_rows_div_zero() {
let mut wb = margins_with_a_zero_revenue_row();
let share = crate::formula::parse_formula("Share = Revenue / SUM(Revenue)", "_Measure");
wb.model.add_formula(share.unwrap());
let (grid, col) = column(wb, "Share");
assert!(grid.cells[2][col].error, "East's row must draw div/0");
let t = &grid.totals[col];
assert_eq!(
(t.text.as_str(), t.error),
("1.00", false),
"60 / 60, pooled"
);
}
}
crates/improvise-command/src/ui/reduce.rs
+15
View File
@@ -654,6 +654,8 @@ pub(crate) mod tests {
/// resolves — otherwise every walk below fails for a reason not its own.
fn model_app() -> App {
let mut app = app();
let workbook = &mut app.model_state.workbook;
workbook.add_item(MEASURE_CATEGORY, "Revenue").unwrap();
app.model_state
.workbook
.model
@@ -959,6 +961,16 @@ pub(crate) mod tests {
]);
wb.model.set_cell(key, CellValue::Number(n as f64 + 1.0));
}
// Several entries, so two instances' HashMaps can disagree on order (improvise-m6op).
use crate::formula::AggFunc;
for (measure, agg) in [
("Revenue", AggFunc::Avg),
("Cost", AggFunc::Min),
("Units", AggFunc::Max),
("Orders", AggFunc::Count),
] {
wb.model.measure_agg.insert(measure.into(), agg);
}
let view = wb.active_view_mut();
view.set_axis("Region", Axis::Row);
view.set_axis("Month", Axis::Column);
@@ -1101,6 +1113,9 @@ pub(crate) mod tests {
},
] {
let mut app = app();
let workbook = &mut app.model_state.workbook;
workbook.add_item(MEASURE_CATEGORY, "Revenue").unwrap();
app.rebuild_layout();
assert!(!app.model_state.dirty, "the fixture starts clean");
let wire = caught_up(&app, cmd.clone());
apply_checked(&mut app, &wire);
crates/improvise-command/src/ui/viewport.rs
+21 -12
View File
@@ -7,7 +7,9 @@ use improvise_protocol::header_level_widths;
use crate::format::{FormatPlan, NumberFormat};
use crate::model::Model;
use crate::model::category::MEASURE_CATEGORY;
use crate::model::cell::{CellKey, CellValue};
use crate::model::types::FoldMemo;
use crate::view::{AxisEntry, GridLayout};
/// Minimum column width — enough for short numbers/labels + 1 char gap.
@@ -26,9 +28,9 @@ pub struct GridBody {
/// The format each column's TOTAL is drawn with, resolved from the
/// coordinates that survive the row aggregation — `context/code/core.md`.
pub col_formats: Vec<NumberFormat>,
/// Column sums over every row. **Empty** in records mode or an empty grid,
/// which is "this grid has no totals" rather than a strip of zeroes.
pub totals: Vec<f64>,
/// Each column's total over every drawn row (improvise-wm20). **Empty** in
/// records mode, an empty grid, or with `_Measure` on the row axis.
pub totals: Vec<Option<CellValue>>,
/// The requested window, row-major. `None` is a cell the layout has not got.
pub window: Vec<Vec<Option<CellValue>>>,
}
@@ -45,9 +47,16 @@ pub fn grid_body(
let n = layout.col_count();
let records = layout.is_records_mode();
let mut widths = header_level_seed(layout, n);
let totalled = !records && layout.row_count() > 0 && n > 0;
let mut totals = match totalled {
true => vec![0.0f64; n],
// One memo for the whole build: a fold every cell and total reaches is
// evaluated once (improvise-h6z1). The model does not change during it.
let memo = FoldMemo::default();
// A column mixing measures has no total to show: `context/code/command.md`.
let measures_on_rows = layout.row_cats.iter().any(|c| c == MEASURE_CATEGORY);
let totalled = !records && !measures_on_rows && layout.row_count() > 0 && n > 0;
let totals: Vec<Option<CellValue>> = match totalled {
true => (0..n)
.map(|ci| layout.column_total(model, ci, &memo))
.collect(),
false => Vec::new(),
};
let mut window: Vec<Vec<Option<CellValue>>> = Vec::with_capacity(rows.len());
@@ -64,16 +73,13 @@ pub fn grid_body(
false => Vec::new(),
};
for ci in 0..n {
let (key, value) = cell_value(model, layout, records, ri, ci);
let (key, value) = cell_value(model, layout, records, ri, ci, &memo);
let fmt = plan.at(key.as_ref());
let text = match records {
true => value.as_ref().map(|v| v.to_string()).unwrap_or_default(),
false => fmt.value(value.as_ref()),
};
widen(&mut widths, ci, text.width());
if let Some(sum) = totals.get_mut(ci) {
*sum += value.as_ref().and_then(|v| v.as_f64()).unwrap_or(0.0);
}
if drawn && cols.contains(&ci) {
row_out.push(value);
}
@@ -84,7 +90,7 @@ pub fn grid_body(
}
for (ci, total) in totals.iter().enumerate() {
let fmt = col_formats.get(ci).copied().unwrap_or(plan.view_format());
widen(&mut widths, ci, fmt.number(*total).width());
widen(&mut widths, ci, fmt.value(total.as_ref()).width());
}
GridBody {
@@ -105,12 +111,15 @@ fn cell_value(
records: bool,
row: usize,
col: usize,
memo: &FoldMemo,
) -> (Option<CellKey>, Option<CellValue>) {
match records {
true => (None, layout.records_value(row, col)),
false => {
let key = layout.cell_key(row, col);
let value = key.as_ref().and_then(|k| layout.evaluate(model, k));
let value = key
.as_ref()
.and_then(|k| layout.evaluate_in(model, k, memo));
(key, value)
}
}
crates/improvise-core/src/model/cell.rs
+175 -7
View File
@@ -378,16 +378,30 @@ impl DataStore {
/// Positions in `cells` of every cell matching `partial`, **in insertion
/// order**. The index narrows; the primary map orders (§8).
fn matching_positions(&self, partial: &[(String, String)]) -> Vec<usize> {
// Intern the partial key (lookup only, no new symbols)
let interned_partial: Vec<(Symbol, Symbol)> = partial
self.intern_partial(partial)
.map(|pairs| self.positions_of(&pairs))
.unwrap_or_default()
}
/// `partial` as symbols, or `None` when a name was never interned, which
/// no stored cell can then match.
fn intern_partial(&self, partial: &[(String, String)]) -> Option<Vec<(Symbol, Symbol)>> {
partial
.iter()
.filter_map(|(c, i)| Some((self.symbols.get(c)?, self.symbols.get(i)?)))
.collect();
.map(|(c, i)| Some((self.symbols.get(c)?, self.symbols.get(i)?)))
.collect()
}
if interned_partial.len() < partial.len() {
return vec![];
}
/// How many candidates narrowing by `pairs` would visit: its smallest index set.
fn narrowing_cost(&self, pairs: &[(Symbol, Symbol)]) -> usize {
pairs
.iter()
.map(|pair| self.index.get(pair).map_or(0, HashSet::len))
.min()
.unwrap_or(self.cells.len())
}
fn positions_of(&self, interned_partial: &[(Symbol, Symbol)]) -> Vec<usize> {
let mut sets: Vec<&HashSet<InternedKey>> = interned_partial
.iter()
.filter_map(|pair| self.index.get(pair))
@@ -426,6 +440,71 @@ impl DataStore {
.collect()
}
/// The union of `matching_values` (or, `exact`, of `get`) over every key `partial`
/// extends by one listed item per spanned category — scanned once (improvise-yb4b).
pub fn spanned_values(
&self,
partial: &[(String, String)],
spans: &[(&str, Vec<&str>)],
exact: bool,
) -> Vec<&CellValue> {
let wanted: Option<Vec<(Symbol, HashSet<Symbol>)>> = spans
.iter()
.map(|(cat, items)| {
let items = items.iter().filter_map(|i| self.symbols.get(i)).collect();
Some((self.symbols.get(cat)?, items))
})
.collect();
let Some(wanted) = wanted else {
return Vec::new();
};
let admits = |ikey: &InternedKey| {
(!exact || ikey.0.len() == partial.len() + wanted.len())
&& wanted
.iter()
.all(|(cat, items)| ikey.0.iter().any(|(c, i)| c == cat && items.contains(i)))
};
self.spanned_positions(partial, &wanted)
.into_iter()
.filter_map(|n| self.cells.get_index(n))
.filter(|(ikey, _)| admits(ikey))
.map(|(_, value)| value)
.collect()
}
/// Candidates for a span: narrowed once by `partial`, or once per covered item
/// of its smallest span, whichever visits fewer. Sorted (improvise-jq0m).
fn spanned_positions(
&self,
partial: &[(String, String)],
wanted: &[(Symbol, HashSet<Symbol>)],
) -> Vec<usize> {
self.intern_partial(partial)
.map(|base| self.narrowed(&base, wanted))
.unwrap_or_default()
}
fn narrowed(
&self,
base: &[(Symbol, Symbol)],
wanted: &[(Symbol, HashSet<Symbol>)],
) -> Vec<usize> {
let with = |cat: Symbol, item: Symbol| [base, &[(cat, item)]].concat();
let per_item: Vec<Vec<(Symbol, Symbol)>> = wanted
.iter()
.min_by_key(|(_, items)| items.len())
.map(|(cat, items)| items.iter().map(|i| with(*cat, *i)).collect())
.unwrap_or_default();
let item_cost: usize = per_item.iter().map(|p| self.narrowing_cost(p)).sum();
let narrow_per_item = !per_item.is_empty() && item_cost < self.narrowing_cost(base);
let mut positions: Vec<usize> = match narrow_per_item {
true => per_item.iter().flat_map(|p| self.positions_of(p)).collect(),
false => self.positions_of(base),
};
positions.sort_unstable();
positions
}
/// Every cell matching `partial`, in insertion order. Allocates a
/// `CellKey` per match — prefer `matching_values` when values suffice.
#[allow(dead_code)]
@@ -769,6 +848,41 @@ mod data_store {
"the fast path and the general path must agree"
);
}
/// improvise-yb4b: one scan answers what a lookup per member answered, in both
/// modes. context/test-evidence/core.md.
#[test]
fn a_spanned_scan_finds_what_a_lookup_per_member_finds() {
let mut store = DataStore::new();
let cells: [(&[(&str, &str)], f64); 7] = [
(&[("M", "Rev"), ("R", "North"), ("P", "A")], 1.0),
(&[("M", "Rev"), ("R", "South"), ("P", "A")], 2.0),
(&[("M", "Rev"), ("R", "East"), ("P", "A")], 4.0),
(&[("M", "Rev"), ("R", "North"), ("P", "A"), ("X", "x")], 8.0),
(&[("M", "Rev"), ("P", "A")], 16.0),
(&[("M", "Cost"), ("R", "North"), ("P", "A")], 32.0),
(&[("M", "Rev"), ("R", "North"), ("P", "B")], 64.0),
];
for (coords, v) in cells {
store.set(key(coords), CellValue::Number(v));
}
let base = key(&[("M", "Rev"), ("P", "A")]);
let covered = vec!["North", "South", "West"];
let members: Vec<CellKey> = covered.iter().map(|r| base.clone().with("R", *r)).collect();
let sum = |values: Vec<&CellValue>| values.iter().filter_map(|v| v.as_f64()).sum::<f64>();
let per_member_exact = members.iter().filter_map(|k| store.get(k)).collect();
let per_member_over = members
.iter()
.flat_map(|k| store.matching_values(&k.0))
.collect();
let spans = [("R", covered.clone())];
let scan_exact = store.spanned_values(&base.0, &spans, true);
let scan_over = store.spanned_values(&base.0, &spans, false);
assert_eq!((sum(scan_exact), sum(per_member_exact)), (3.0, 3.0));
assert_eq!((sum(scan_over), sum(per_member_over)), (11.0, 11.0));
let unknown = [("Nowhere", covered)];
assert!(store.spanned_values(&base.0, &unknown, false).is_empty());
}
}
#[cfg(test)]
@@ -941,6 +1055,60 @@ mod prop_tests {
"returned key {result_key} does not match partial {partial:?}");
}
}
/// improvise-jq0m: whichever narrowing it picks, a span returns what a lookup
/// per member returns, in store order. context/test-evidence/core.md.
#[test]
fn a_span_finds_what_its_members_find_in_store_order(
cells in prop::collection::vec((0u8..6, 0u8..3, 0u8..2, any::<bool>()), 0..40),
covered_r in prop::collection::vec(any::<bool>(), 7),
covered_p in prop::option::of(prop::collection::vec(any::<bool>(), 3)),
base_m in 0u8..2,
exact in any::<bool>(),
) {
let mut store = DataStore::new();
for (n, (r, p, m, extra)) in cells.iter().enumerate() {
let mut key = CellKey::new(vec![
("P".into(), format!("p{p}")),
("M".into(), format!("m{m}")),
]);
// r = 5 leaves the record without the spanned category at all.
key = match r { 5 => key, r => key.with("R", format!("r{r}")) };
key = match extra { true => key.with("X", "x"), false => key };
store.set(key, CellValue::Number(n as f64));
}
// r6 is never stored, so it is a covered item the store never interned.
let pick = |prefix: &str, flags: &[bool]| -> Vec<String> {
flags.iter().enumerate().filter(|(_, f)| **f)
.map(|(i, _)| format!("{prefix}{i}")).collect()
};
let rs = pick("r", &covered_r);
let ps = covered_p.as_deref().map(|f| pick("p", f));
let base = CellKey::new(vec![("M".into(), format!("m{base_m}"))]);
let mut spans: Vec<(&str, Vec<&str>)> = vec![("R", rs.iter().map(String::as_str).collect())];
if let Some(ps) = &ps {
spans.push(("P", ps.iter().map(String::as_str).collect()));
}
let mut members = vec![base.clone()];
for (cat, items) in &spans {
members = members.iter()
.flat_map(|k| items.iter().map(|i| k.clone().with(*cat, *i)))
.collect();
}
let matched: std::collections::HashSet<CellKey> = members.iter()
.flat_map(|k| match exact {
true => store.get(k).map(|_| k.clone()).into_iter().collect::<Vec<_>>(),
false => store.matching_cells(&k.0).into_iter().map(|(k, _)| k).collect(),
})
.collect();
let expected: Vec<CellValue> = store.iter_cells()
.filter(|(k, _)| matched.contains(k))
.map(|(_, v)| v.clone())
.collect();
let got: Vec<CellValue> = store.spanned_values(&base.0, &spans, exact)
.into_iter().cloned().collect();
prop_assert_eq!(got, expected);
}
}
}
crates/improvise-core/src/model/types.rs
+148 -59
View File
@@ -140,17 +140,62 @@ fn agg_values(
// How many formulas a fold has evaluated: what improvise-o1y0's regression counts.
#[cfg(test)]
thread_local! {
static FOLD_FORMULA_EVALUATIONS: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
pub(crate) static FOLD_FORMULA_EVALUATIONS: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
}
/// One fold evaluation's memory: each formula key is evaluated once, and a key met
/// while still in progress (`None`) is a cycle (improvise-o1y0).
type FoldMemo = std::cell::RefCell<HashMap<CellKey, Option<Option<CellValue>>>>;
/// Each (fold key, span) evaluated once across one grid build (improvise-o1y0,
/// improvise-h6z1). Valid for one model state only: `context/code/core.md` §14.
#[derive(Default)]
pub struct FoldMemo(std::cell::RefCell<HashMap<(CellKey, Vec<String>), MemoEntry>>);
/// `None` while in progress, which is how a cycle is seen (improvise-o1y0).
type MemoEntry = Option<Option<CellValue>>;
impl FoldMemo {
fn get(&self, key: &CellKey, span: Span) -> Option<MemoEntry> {
let at = (key.clone(), span.over.to_vec());
self.0.borrow().get(&at).cloned()
}
fn put(&self, key: &CellKey, span: Span, entry: MemoEntry) {
let at = (key.clone(), span.over.to_vec());
self.0.borrow_mut().insert(at, entry);
}
}
/// Which members of a collapsed group its fold covers. The model knows no
/// view, so the grid passes the members it draws (core.md §14).
pub type Covers<'a> = &'a dyn Fn(&str, &str) -> bool;
/// What a fold reaches: the covered members of each group its key names, and
/// every covered item of each category in `over` its key omits (improvise-wm20).
#[derive(Clone, Copy)]
struct Span<'a> {
covers: Covers<'a>,
over: &'a [String],
}
/// A fold's reach as data: `base` is the key with each expanded category removed,
/// and `spans` the covered items of each, in expansion order (improvise-yb4b).
struct Expansion<'a> {
base: CellKey,
spans: Vec<(&'a str, Vec<&'a str>)>,
}
impl Expansion<'_> {
/// One key per member, the cross product in expansion order.
fn members(&self) -> Vec<CellKey> {
let mut expanded = vec![self.base.clone()];
for (cat_name, covered) in &self.spans {
expanded = expanded
.iter()
.flat_map(|k| covered.iter().map(|m| k.clone().with(*cat_name, *m)))
.collect();
}
expanded
}
}
/// Where a formula's references are read: at one record, or across the members
/// a collapsed group's fold covers, as a None axis reads them (core.md §14).
#[derive(Clone, Copy)]
@@ -158,7 +203,7 @@ enum Reach<'a> {
Record,
Fold {
agg: Aggregation,
covers: Covers<'a>,
span: Span<'a>,
memo: &'a FoldMemo,
},
}
@@ -184,9 +229,7 @@ fn eval_expr(
let new_key = context.clone().with(cat, name);
let value = match reach {
Reach::Record => model.evaluate_depth(&new_key, depth),
Reach::Fold { agg, covers, memo } => {
model.evaluate_fold(&new_key, agg, covers, memo)
}
Reach::Fold { agg, span, memo } => model.evaluate_fold(&new_key, agg, span, memo),
};
match value {
Some(CellValue::Number(n)) => Ok(n),
@@ -654,7 +697,46 @@ impl Model {
agg: Aggregation,
covers: Covers,
) -> Option<CellValue> {
self.evaluate_fold(key, agg, covers, &FoldMemo::default())
let span = Span { covers, over: &[] };
self.evaluate_fold(key, agg, span, &FoldMemo::default())
}
/// [`Self::evaluate_covering`] sharing `memo` with every other evaluation of one
/// grid build, which must not outlive the model state it read (improvise-h6z1).
pub fn evaluate_covering_in(
&self,
key: &CellKey,
agg: Aggregation,
covers: Covers,
memo: &FoldMemo,
) -> Option<CellValue> {
let span = Span { covers, over: &[] };
self.evaluate_fold(key, agg, span, memo)
}
/// The value `key` has as ONE fold over every covered item of the `over`
/// categories it omits — a column's total (improvise-wm20, core.md §14).
pub fn evaluate_spanning(
&self,
key: &CellKey,
over: &[String],
agg: Aggregation,
covers: Covers,
) -> Option<CellValue> {
self.evaluate_spanning_in(key, over, agg, covers, &FoldMemo::default())
}
/// [`Self::evaluate_spanning`] sharing one grid build's `memo` (improvise-h6z1).
pub fn evaluate_spanning_in(
&self,
key: &CellKey,
over: &[String],
agg: Aggregation,
covers: Covers,
memo: &FoldMemo,
) -> Option<CellValue> {
let span = Span { covers, over };
self.evaluate_fold(key, agg, span, memo)
}
/// A key naming a group folds; any other key reads as before. A formula is
@@ -663,16 +745,16 @@ impl Model {
&self,
key: &CellKey,
agg: Aggregation,
covers: Covers,
span: Span,
memo: &FoldMemo,
) -> Option<CellValue> {
let members = self.group_expansion(key, covers);
match (members.is_empty(), self.formula_for(key)) {
let expansion = self.group_expansion(key, span);
match (expansion.spans.is_empty(), self.formula_for(key)) {
(true, _) => self.evaluate_unfolded(key, agg),
(false, Some(formula)) => {
self.fold_formula_once(formula, key, &members, (agg, covers, memo))
self.fold_formula_once(formula, key, &expansion, (agg, span, memo))
}
(false, None) => self.fold_stored(key, &members, agg),
(false, None) => self.fold_stored(key, &expansion, agg),
}
}
@@ -682,20 +764,20 @@ impl Model {
&self,
formula: &Formula,
key: &CellKey,
members: &[CellKey],
(agg, covers, memo): (Aggregation, Covers, &FoldMemo),
expansion: &Expansion,
(agg, span, memo): (Aggregation, Span, &FoldMemo),
) -> Option<CellValue> {
let seen = memo.borrow().get(key).cloned();
let seen = memo.get(key, span);
match seen {
Some(Some(value)) => value,
Some(None) => Some(CellValue::Error("circular".into())),
None => {
memo.borrow_mut().insert(key.clone(), None);
memo.put(key, span, None);
#[cfg(test)]
FOLD_FORMULA_EVALUATIONS.with(|n| n.set(n.get() + 1));
let reach = Reach::Fold { agg, covers, memo };
let value = self.eval_formula_folded(formula, key, members, agg, reach);
memo.borrow_mut().insert(key.clone(), Some(value.clone()));
let reach = Reach::Fold { agg, span, memo };
let value = self.eval_formula_folded(formula, key, expansion, agg, reach);
memo.put(key, span, Some(value.clone()));
value
}
}
@@ -712,31 +794,40 @@ impl Model {
aggregate_cell_values(&self.data.matching_values(&key.0), self.agg_for(key))
}
/// One key per covered member where `key` names a collapsed GROUP rather than
/// an item — the cross product over every such coordinate. Empty when none does.
fn group_expansion(&self, key: &CellKey, covers: Covers) -> Vec<CellKey> {
let mut expanded = vec![key.clone()];
for (cat_name, name) in &key.0 {
/// Each category `key` folds, with its covered members: a collapsed GROUP it
/// names, or a spanned category it omits. No spans when neither.
fn group_expansion<'a>(&'a self, key: &'a CellKey, span: Span<'a>) -> Expansion<'a> {
let grouped = key.0.iter().map(|(cat_name, name)| {
let members: Vec<&str> = self
.category(cat_name)
.filter(|cat| !cat.items.contains_key(name.as_str()))
.map(|cat| cat.items_in_group(name))
.unwrap_or_default()
.into_iter()
.filter(|m| covers(cat_name, m))
.collect();
if members.is_empty() {
continue;
}
expanded = expanded
.iter()
.flat_map(|k| members.iter().map(|m| k.clone().with(cat_name, *m)))
.collect();
}
match expanded.as_slice() {
[only] if only == key => Vec::new(),
_ => expanded,
}
.unwrap_or_default();
(cat_name.as_str(), members)
});
let spanned = span
.over
.iter()
.filter(|cat_name| key.get(cat_name).is_none())
.map(|cat_name| {
let items = self.category(cat_name).map(|cat| cat.ordered_item_names());
(cat_name.as_str(), items.unwrap_or_default())
});
let spans: Vec<(&str, Vec<&str>)> = grouped
.chain(spanned)
.map(|(cat_name, members)| {
let covered: Vec<&str> = members
.into_iter()
.filter(|m| (span.covers)(cat_name, m))
.collect();
(cat_name, covered)
})
.filter(|(_, covered)| !covered.is_empty())
.collect();
let base = spans
.iter()
.fold(key.clone(), |k, (cat_name, _)| k.without(cat_name));
Expansion { base, spans }
}
/// A coordinate as a formula reads it. At a fold a group's name is no item, so
@@ -762,30 +853,28 @@ impl Model {
/// The contexts an aggregate inside a formula pools over at `context`.
fn contexts_of(&self, context: &CellKey, reach: Reach) -> Vec<CellKey> {
let members = match reach {
Reach::Record => Vec::new(),
Reach::Fold { covers, .. } => self.group_expansion(context, covers),
let expansion = match reach {
Reach::Record => None,
Reach::Fold { span, .. } => Some(self.group_expansion(context, span)),
};
match members.is_empty() {
true => vec![context.clone()],
false => members,
match expansion.filter(|e| !e.spans.is_empty()) {
None => vec![context.clone()],
Some(e) => e.members(),
}
}
/// A group's stored values fold ONCE over every record the group covers.
/// A group's stored values fold ONCE over every record the group covers, in
/// the store's own order: one scan, not a lookup per member (improvise-yb4b).
fn fold_stored(
&self,
key: &CellKey,
members: &[CellKey],
expansion: &Expansion,
agg: Aggregation,
) -> Option<CellValue> {
let values: Vec<&CellValue> = members
.iter()
.flat_map(|k| match agg {
Aggregation::Exact => self.data.get(k).into_iter().collect(),
Aggregation::OverHidden => self.data.matching_values(&k.0),
})
.collect();
let exact = agg == Aggregation::Exact;
let values = self
.data
.spanned_values(&expansion.base.0, &expansion.spans, exact);
aggregate_cell_values(&values, self.agg_for(key))
}
@@ -795,7 +884,7 @@ impl Model {
&self,
formula: &Formula,
key: &CellKey,
members: &[CellKey],
expansion: &Expansion,
agg: Aggregation,
reach: Reach,
) -> Option<CellValue> {
@@ -804,7 +893,7 @@ impl Model {
.as_ref()
.is_none_or(|f| self.read_coordinate(key, &f.category, reach) == Some(f.item.as_str()));
if !applies {
return self.fold_stored(key, members, agg);
return self.fold_stored(key, expansion, agg);
}
let depth = Self::MAX_EVAL_DEPTH;
match eval_expr(
crates/improvise-core/src/view/layout.rs
+57 -1
View File
@@ -3,6 +3,7 @@ use std::rc::Rc;
use crate::model::category::{DIM_CATEGORY, INDEX_CATEGORY, MEASURE_CATEGORY};
use crate::model::cell::{CellKey, CellValue};
use crate::model::types::FoldMemo;
use crate::model::{Aggregation, Model};
use crate::view::{Axis, View};
@@ -657,8 +658,22 @@ impl GridLayout {
/// The value of a pivot key as THIS grid reads it: aggregated over its None
/// axis, and a fold over the members it draws (core.md §14). One evaluation.
pub fn evaluate(&self, model: &Model, key: &CellKey) -> Option<CellValue> {
self.evaluate_in(model, key, &FoldMemo::default())
}
/// [`Self::evaluate`] sharing one grid build's `memo` (improvise-h6z1).
pub fn evaluate_in(&self, model: &Model, key: &CellKey, memo: &FoldMemo) -> Option<CellValue> {
let drawn = |cat: &str, item: &str| !self.hidden.get(cat).is_some_and(|h| h.contains(item));
model.evaluate_covering(key, Aggregation::of(&self.none_cats), &drawn)
model.evaluate_covering_in(key, Aggregation::of(&self.none_cats), &drawn, memo)
}
/// A column's total: its cell as if every drawn row were ONE fold, so a
/// formula is evaluated over the pooled rows (improvise-wm20, core.md §14).
pub fn column_total(&self, model: &Model, col: usize, memo: &FoldMemo) -> Option<CellValue> {
let key = self.column_key(col)?;
let drawn = |cat: &str, item: &str| !self.hidden.get(cat).is_some_and(|h| h.contains(item));
let agg = Aggregation::of(&self.none_cats);
model.evaluate_spanning_in(&key, &self.row_cats, agg, &drawn, memo)
}
/// Display text for (row, col) in either mode.
@@ -895,6 +910,7 @@ mod tests {
prune_axis_items, synthetic_record_info,
};
use crate::model::cell::{CellKey, CellValue};
use crate::model::types::{FOLD_FORMULA_EVALUATIONS, FoldMemo};
use crate::view::Axis;
use crate::workbook::Workbook;
@@ -1832,6 +1848,46 @@ mod tests {
.and_then(|v| v.as_f64())
}
/// improvise-h6z1: one grid build evaluates each formula a fold row reaches
/// once, not once per column. context/test-evidence/core.md.
#[test]
fn a_fold_row_evaluates_each_formula_once_across_the_row() {
let chain: Vec<String> = std::iter::once("F0 = Revenue".to_string())
.chain((1..=8).map(|i| format!("F{i} = F{p} + F{p}", p = i - 1)))
.collect();
let chain: Vec<&str> = chain.iter().map(String::as_str).collect();
let mut wb = margin_book(&chain);
wb.active_view_mut().toggle_group_collapse("Month", "Q1");
wb.model.recompute_formulas(&wb.active_view().none_cats());
let layout = GridLayout::new(&wb.model, wb.active_view());
let q1 = (0..layout.row_count())
.find(|&r| layout.row_label(r) == "Q1")
.expect("a Q1 fold row");
let row_keys: Vec<CellKey> = (0..layout.col_count())
.filter_map(|c| layout.cell_key(q1, c))
.collect();
let evaluations = |shared: bool| {
let memo = FoldMemo::default();
FOLD_FORMULA_EVALUATIONS.with(|n| n.set(0));
let values: Vec<_> = row_keys
.iter()
.map(|k| match shared {
true => layout.evaluate_in(&wb.model, k, &memo),
false => layout.evaluate(&wb.model, k),
})
.collect();
(FOLD_FORMULA_EVALUATIONS.with(|n| n.get()), values)
};
let (apart, separately) = evaluations(false);
let (together, shared) = evaluations(true);
assert_eq!(shared, separately, "sharing the memo changed a value");
assert_eq!(
apart, 45,
"setup: each column re-derives the chain below it"
);
assert_eq!(together, 9, "F0..F8 once each across the whole row");
}
/// improvise-eccl — see context/test-evidence/core.md.
#[test]
fn a_folds_ratio_formula_is_the_ratio_of_its_folded_inputs() {
crates/improvise-formula/src/parser.rs
+37 -8
View File
@@ -1167,9 +1167,23 @@ mod generator {
use pest_meta::parser;
use proptest::prelude::*;
use std::collections::HashMap;
use std::sync::OnceLock;
use std::sync::atomic::{AtomicUsize, Ordering};
/// Counts how often formula.pest is actually parsed; the guard below keeps it
/// from becoming a per-case parse again.
static GRAMMAR_PARSES: AtomicUsize = AtomicUsize::new(0);
/// The parsed grammar, read once for the process. Evidence:
/// `context/test-evidence/formula-and-io.md`.
fn grammar() -> &'static HashMap<String, (RuleType, Expr)> {
static RULES: OnceLock<HashMap<String, (RuleType, Expr)>> = OnceLock::new();
RULES.get_or_init(load_grammar)
}
/// Parse formula.pest and return rules keyed by name.
fn load_grammar() -> HashMap<String, (RuleType, Expr)> {
GRAMMAR_PARSES.fetch_add(1, Ordering::Relaxed);
let grammar = include_str!("formula.pest");
let pairs = parser::parse(parser::Rule::grammar_rules, grammar)
.unwrap_or_else(|e| panic!("Bad grammar: {e}"));
@@ -1301,18 +1315,33 @@ mod generator {
/// Proptest strategy: generate a random valid formula by walking the
/// grammar's `formula` rule.
pub fn formula_string() -> impl Strategy<Value = String> {
prop::collection::vec(any::<u8>(), 32..=128).prop_map(|choices| {
let rules = load_grammar();
Gen::new(&rules, choices).generate("formula")
})
prop::collection::vec(any::<u8>(), 32..=128)
.prop_map(|choices| Gen::new(grammar(), choices).generate("formula"))
}
/// Proptest strategy: generate a random valid standalone expression.
pub fn expr_string() -> impl Strategy<Value = String> {
prop::collection::vec(any::<u8>(), 32..=128).prop_map(|choices| {
let rules = load_grammar();
Gen::new(&rules, choices).generate("expr_eoi")
})
prop::collection::vec(any::<u8>(), 32..=128)
.prop_map(|choices| Gen::new(grammar(), choices).generate("expr_eoi"))
}
/// improvise-i5z6. Evidence: `context/test-evidence/formula-and-io.md`.
#[test]
fn the_grammar_is_parsed_once_however_many_formulas_the_strategies_yield() {
use proptest::strategy::ValueTree;
use proptest::test_runner::TestRunner;
let mut runner = TestRunner::deterministic();
let (formulas, exprs) = (formula_string(), expr_string());
for _ in 0..32 {
let _ = formulas.new_tree(&mut runner).unwrap().current();
let _ = exprs.new_tree(&mut runner).unwrap().current();
}
assert_eq!(
GRAMMAR_PARSES.load(Ordering::Relaxed),
1,
"formula.pest was re-parsed per generated case"
);
}
}
crates/improvise-protocol/src/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
+23 -3
View File
@@ -133,14 +133,34 @@ 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. `head`
# and `env` are for 5b's 5.1 control, which runs past the version check.
mkdir -p "$work/utilbin"
for tool in dirname head env; 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:$work/utilbin" 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)
);
}
}