Merge remote-tracking branch 'gh/main' into fix/headless-exit-extent
This commit is contained in:
Edward Langley
@@ -7,6 +7,8 @@ description: Dispatch parallel agents to work through open beads and shepherd ea
|
||||
|
||||
An orchestrator chunks the open backlog, fans agents out, reviews what comes
|
||||
back, and merges. `AGENTS.md` has the history; this is the operating procedure.
|
||||
This file is the brief checklist's one home, and `agent-rules.md` beside it
|
||||
holds the rules each agent follows.
|
||||
|
||||
## Every agent gets its own worktree. No exceptions.
|
||||
|
||||
@@ -17,9 +19,8 @@ here. If you discover it mid-wave, stop every agent, have each create a worktree
|
||||
and copy across **only files it can prove it touched**, then `git checkout --`
|
||||
those files in the primary tree.
|
||||
|
||||
In the worktree: `direnv allow`, or `bd` reports a version *mismatch* rather than
|
||||
failing cleanly. `direnv status` prints `Found RC allowed 0`, and `0` means
|
||||
allowed — it reads like the opposite.
|
||||
In the worktree `.envrc` must be allowed, or `bd` fails confusingly:
|
||||
`agent-rules.md`, "Discipline per ticket".
|
||||
|
||||
## Partitioning
|
||||
|
||||
@@ -33,32 +34,41 @@ it must cross. Two agents adding a workspace member will still collide in
|
||||
|
||||
## What every brief must carry
|
||||
|
||||
**Point the agent at [`agent-rules.md`](agent-rules.md) and tell it to read it in
|
||||
full first.** That file is the agent's standing rules: base and push mechanics,
|
||||
the gate list, the claim, the vacuity shapes, evidence, the files it must not
|
||||
write, GitHub conventions, the hand-back and disk cleanup. Do not restate them
|
||||
in the brief. A restated rule drifts from its source, and the brief is the copy
|
||||
nobody corrects. The brief adds only what varies per wave:
|
||||
|
||||
1. **The expected base commit, by name.** Worktrees do not reliably branch from
|
||||
HEAD — one was created three merges behind and only noticed because it was
|
||||
told to check.
|
||||
2. **The baseline**, so "N passing" is checkable rather than asserted: Rust test
|
||||
count, harness counts, node counts.
|
||||
3. **Its ground, and the ground that is not.**
|
||||
4. **Never write** `.beads/**`, `prolog-sessions/kb.pl`, `context/repo-map.md`.
|
||||
Agents read them freely and report proposed text; the orchestrator applies it.
|
||||
5. **The commit trailers and PR footer**, spelled out.
|
||||
6. **Do not push to `main`, do not merge.** The orchestrator merges.
|
||||
7. **What to report**: branch and head SHA, PR number, what changed per ticket,
|
||||
*what could not be verified and why*, proposed repo-map/kb text, and **what it
|
||||
learned that the ticket did not predict** — consistently the most valuable
|
||||
part, and omitted unless asked for explicitly.
|
||||
HEAD. One was created three merges behind and was only noticed because it
|
||||
was told to check. The agent checks against it: `agent-rules.md` rule 1.
|
||||
2. **A reference baseline**, so "N passing" is checkable and a drop is
|
||||
visible: Rust test count, harness counts, node counts, **and the machine**.
|
||||
The same commit once measured `improvise-io`'s binary at 1.4s on one machine
|
||||
and 3.7–5.2s on another, so a figure with no machine is a claim about the
|
||||
suite, which it is not. The agent still measures its own.
|
||||
3. **Its ground, and the ground that is not.** Name any orchestrator-only file
|
||||
it is exceptionally allowed to edit.
|
||||
4. **Its `BEADS_ACTOR` and its tickets.**
|
||||
5. **The attribution line, commit trailers and PR footer**, spelled out.
|
||||
They name the agent (Claude's line opens `_Posted by Claude (<model>)…_`)
|
||||
and this session's URL, which is why the shared `agent-rules.md` cannot
|
||||
hold them.
|
||||
6. **An explicit ask for what the ticket did not predict.** `agent-rules.md`
|
||||
lists it in the hand-back. Say it in the brief anyway: it is consistently
|
||||
the most valuable part of a hand-back, and agents omit it unless asked.
|
||||
|
||||
## Method to require, because this repo's defect is a test that passes
|
||||
When a hand-back teaches a new rule, add it to `agent-rules.md` in the same
|
||||
wave, not to the next brief.
|
||||
|
||||
- Failing test first, then the fix.
|
||||
- Every guard proved by **injecting the fault it names** and watching it fail
|
||||
**by name**. An injection that does not compile reads as a clean run, so
|
||||
require a positive signal the build ran.
|
||||
- `cargo test --workspace` is **fail-fast**; `--no-fail-fast` for any claim about
|
||||
blast radius.
|
||||
- A textually clean merge can **delete tests** with no conflict marker, and
|
||||
`cargo check` will not notice. After any merge that moves a test module, diff
|
||||
test **names** — a count is masked by the other side's additions.
|
||||
## Method to require
|
||||
|
||||
`agent-rules.md`, "Discipline per ticket" and "Gates": failing test first, a
|
||||
rival injected and watched failing by name, `--no-fail-fast`, and test *names*
|
||||
diffed after any merge. Hold the hand-back to it. This repo's characteristic
|
||||
defect is a test that passes.
|
||||
|
||||
## Shepherding
|
||||
|
||||
@@ -72,7 +82,9 @@ it must cross. Two agents adding a workspace member will still collide in
|
||||
commit reads as a pass.
|
||||
- Merge on GitHub, then `git fetch gh && git merge --ff-only gh/main`.
|
||||
- Anything posted to GitHub goes out under the owner's account: open every
|
||||
comment with a line saying a Codex agent wrote it.
|
||||
comment with a line naming the model and saying an agent wrote it. That
|
||||
wording is agent-neutral because this one file serves Claude and Codex
|
||||
(`.claude/skills/wave` is a symlink to `.agents/skills/wave`).
|
||||
|
||||
## Between waves
|
||||
|
||||
|
||||
@@ -0,0 +1,198 @@
|
||||
# Standing rules for a wave agent
|
||||
|
||||
Every work-unit agent reads this **in full before its first command**. A brief
|
||||
points here and adds only what is specific to its wave: the expected base
|
||||
commit, the agent's ground, its `BEADS_ACTOR`, and its tickets. Every rule below
|
||||
was bought by a failure.
|
||||
|
||||
This file holds each agent-side rule **once**. The brief checklist is
|
||||
`SKILL.md` beside it. Where `AGENTS.md` ("The Wave Workflow") carries a longer
|
||||
history, the rule here points there. Change a rule here, not in a brief, and
|
||||
not in a second copy.
|
||||
|
||||
## Base, branch, push
|
||||
|
||||
1. **Verify your base against the commit your brief names, first.** Run
|
||||
`git log --oneline -1` and compare. Worktrees do not reliably branch from
|
||||
the orchestrator's HEAD: one was created three merges behind, and it was
|
||||
only noticed because the agent was told to look.
|
||||
2. **Branch from `gh/main`, never `main` or `origin/main`.** `git fetch gh &&
|
||||
git checkout -b <branch> gh/main`. A worktree handed to you may already sit
|
||||
on unpushed commits. Why: `AGENTS.md`, "Shipping a wave" (the two remotes
|
||||
both spelled `main`, and the worktree-base vector).
|
||||
3. **Before opening a PR, check `git diff --numstat gh/main...<branch>`.** Do
|
||||
not use `gh pr view --json files`: that is the union of your commits' files,
|
||||
not the effective diff.
|
||||
4. **Push with an explicit refspec:** `git push gh HEAD:refs/heads/<branch>`. A
|
||||
branch cut from `gh/main` tracks `gh/main`, so a bare push targets `main` and
|
||||
is refused.
|
||||
5. **Before pushing a fix to an existing PR, check `gh pr view <n> --json
|
||||
state`.** GitHub deletes a merged PR's branch. A push to it then prints
|
||||
`* [new branch]` and lands nowhere. Why: `AGENTS.md`, "Shipping a wave".
|
||||
6. **The "Session Completion" block near the end of `AGENTS.md` does not apply
|
||||
to you.** It says `git merge origin/main` and then a bare `git push`. On a
|
||||
wave branch the first is the #145 trap and the second is refused (rule 4).
|
||||
Rules 1–5 replace it. The block sits between bd's generated
|
||||
`BEGIN/END BEADS INTEGRATION` markers, so it is not edited by hand
|
||||
(improvise-x1uk).
|
||||
|
||||
## Commit early: a worktree is not durable storage
|
||||
|
||||
Commit a WIP as soon as your first test run is green, and again after every
|
||||
meaningful step. Push your branch (rule 4) once it is worth keeping. An agent
|
||||
finished a whole ticket with every gate green and lost all of it, uncommitted,
|
||||
when its worktree was deleted in a disk cleanup. A local commit survives a
|
||||
deleted worktree. Tidying history before the PR is fine; losing it is not.
|
||||
|
||||
## Gates
|
||||
|
||||
```
|
||||
direnv exec . cargo test --workspace --no-fail-fast
|
||||
direnv exec . cargo clippy --workspace --all-targets -- -D warnings
|
||||
direnv exec . ./scripts/fmt-workspaces.sh # NOT cargo fmt --all; exit 2 = the gate did not run
|
||||
direnv exec . python3 scripts/check-comment-runs.py --check-evidence
|
||||
direnv exec . ./scripts/checkers.sh
|
||||
direnv exec . ./scripts/node-test.sh 'web/tests/*.test.js'
|
||||
```
|
||||
|
||||
- **Never `cargo fmt --all`.** It reaches only **one** of this repo's two
|
||||
workspace roots, because `harness/opfs-contract` declares its own
|
||||
`[workspace]`. The obvious repair is worse: `cargo fmt -p opfs-contract` from
|
||||
the repo root exits **1**, *"not a member of the workspace"*, which looks
|
||||
exactly like a real formatting failure (improvise-b4ks). Both spellings fail
|
||||
in the direction that looks like success.
|
||||
- **Run every gate under `direnv exec .`.** `./scripts/checkers.sh` scrubs the
|
||||
environment to an allowlist, so a bare run drops `tpl` off `PATH` and reports
|
||||
spurious failures.
|
||||
- **Never pipe a gate.** `$?` must be the gate's own. And
|
||||
`check-comment-runs.py --check-evidence` prints two lines with the comment
|
||||
cap **first**, so `| tail -1` silently drops half of it.
|
||||
- **`--no-fail-fast`**, because plain `cargo test --workspace` stops at the
|
||||
first failing binary. Use it for any claim about blast radius.
|
||||
- If you touch a browser-bundle crate (core, formula, io, protocol, command,
|
||||
wasm-client, worker-server), also run
|
||||
`cargo check --target wasm32-unknown-unknown -p <crate>` **for each crate**.
|
||||
CI checks each one separately, so feature unification cannot hide a leak.
|
||||
- If you touch `web/` or `harness/`, run the Playwright suite in `harness/`.
|
||||
- **Measure your own baseline** before changing anything, and report it with
|
||||
the machine it was measured on. A figure from the brief is a reference, not
|
||||
your baseline.
|
||||
- **Merge `gh/main` and re-run every gate before calling a PR green.** Then
|
||||
check that no test *disappeared*. Diff test **names**, not counts: a
|
||||
textually clean merge can delete a test module with no conflict marker, and
|
||||
the other side's new tests mask the loss in a count.
|
||||
- **The shell guard refusing a compound command means NEITHER part ran.** Split
|
||||
the command. After a refusal, re-check the state it was meant to produce.
|
||||
- **Re-run the full suite after reverting an injected fault.** A revert applied
|
||||
to the wrong occurrence once shipped two faults past every other gate.
|
||||
|
||||
## Discipline per ticket
|
||||
|
||||
- **Claim first:** `export BEADS_ACTOR=<your actor>` in every shell, then
|
||||
`bd update <id> --claim`. Exit 13 means another actor holds it. Pick another;
|
||||
do not retry.
|
||||
- **`bd` needs `.envrc` allowed in your worktree.** Without it you get a
|
||||
**version mismatch**, not a clean failure. Run `direnv allow` in the worktree,
|
||||
or rely on the `[whitelist] prefix` in `~/.config/direnv/direnv.toml`.
|
||||
`direnv status` reports `Found RC allowed 0`, and `0` means **allowed** (the
|
||||
enum is 0=Allowed, 1=NotAllowed). It reads like the opposite.
|
||||
- **Reproduce at HEAD before fixing.** A dozen tickets were stale-open in one
|
||||
campaign, one fixed two days before it was filed. If it does not reproduce,
|
||||
find the fixing commit, confirm with `git merge-base --is-ancestor <sha> HEAD`,
|
||||
report the SHA, and close nothing.
|
||||
- **Write a failing test first, then inject a rival and watch it fail.** Run
|
||||
it; do not reason about it. Report *where* it failed, by case name. An
|
||||
injection that does not compile reads as a clean run, so get a positive
|
||||
signal that the build ran.
|
||||
- **Check your test against every vacuity shape seen here:**
|
||||
- an assertion the default state already satisfies;
|
||||
- `f(x) == f(x)`: calling a pure `Cmd` twice against one immutable context;
|
||||
- a disjunctive assertion whose other branch is the bug's own symptom;
|
||||
- a rival that fails the *setup*, which proves the neighbouring question
|
||||
(e.g. setup and subject share an `EffectKind`);
|
||||
- a fixture that is right by accident (expected value 1, group at row 0,
|
||||
bystander already on the relayed value);
|
||||
- a fixture or stub that lacks the structure the hazard needs, or fails in a
|
||||
wider shape than the real fault;
|
||||
- hand-picked fixture cases where the set could be **enumerated**: four
|
||||
vacuities in `fmt-workspaces-test.sh` were each a case nobody thought of
|
||||
(improvise-90ea);
|
||||
- a rendered-output test at a width where the thing asserted is never drawn;
|
||||
- a fold test that batches differently from the real producer (a
|
||||
`Binding::Sequence` is ONE effect vec);
|
||||
- **the cursor at index 0**: "the item under the cursor" and "the first item"
|
||||
are indistinguishable there, and that was four of five findings in one
|
||||
review. Seat the cursor at a nonzero index of a list with at least two
|
||||
elements;
|
||||
- a determinism check against ONE context instance: every `HashMap` iterates
|
||||
the same way twice. Build two equal contexts independently
|
||||
(improvise-hzjo);
|
||||
- two derivations of one fact where the fixture makes them agree (drawn vs
|
||||
committed, edit text vs value, `1200` vs `1.23456789`). Use a value where
|
||||
they disagree.
|
||||
- **Record each bug in `context/test-evidence/<area>.md`**, keyed by test name
|
||||
(module-qualify the name when the leaf is not unique). It is append-at-EOF, so
|
||||
**expect a conflict**. Keep both sides and verify line by line against both
|
||||
parents: `--check-evidence` cannot see a dropped entry.
|
||||
- **Comment runs are at most two lines.** Reasoning goes in `context/`, not in
|
||||
comments (`context/design-principles.md` §6).
|
||||
- **If the acceptance criteria look wrong, argue it** rather than following
|
||||
them silently. Three agents inverted theirs in one campaign, and all three
|
||||
were right.
|
||||
|
||||
## Files you must not write
|
||||
|
||||
`.beads/**` (never commit `issues.jsonl`, never `git checkout` it),
|
||||
`prolog-sessions/kb.pl`, `context/repo-map.md`: `AGENTS.md`, "Who owns what".
|
||||
Also `AGENTS.md`, unless your brief grants you a named section. Report proposed
|
||||
text for any of these in your hand-back.
|
||||
|
||||
## GitHub
|
||||
|
||||
- `gh` is shell-aliased to a different tool. Use
|
||||
`nix shell nixpkgs#gh --command gh …`, and pass bodies with `-F body=@file`.
|
||||
- **Everything posted goes out under the repository owner's account.** Open
|
||||
every PR body and comment with the attribution line your brief gives you,
|
||||
verbatim. It names your agent and model, which is how our posts are told
|
||||
apart from an automated Codex review (`AGENTS.md`, "Shipping a wave").
|
||||
- The attribution line, commit trailers and PR footer all come from your brief.
|
||||
They name the agent and the session, so this shared file cannot hold them.
|
||||
- Read reviews with `./scripts/pr-reviews <n>`, and do not pipe it. How to tell
|
||||
a ChatGPT Codex review from ours, and why Copilot is not coming: `AGENTS.md`,
|
||||
"Shipping a wave". Answer each Codex review on its thread.
|
||||
- **Do not merge, and do not close beads.** Avoid stacked PRs. If you must
|
||||
stack, never let the base branch be deleted.
|
||||
|
||||
## Hand-back, per ticket
|
||||
|
||||
Report:
|
||||
|
||||
- your branch, head SHA and PR number. Review is dispatched against the SHA;
|
||||
- what changed, per ticket;
|
||||
- the claim result;
|
||||
- your measured baseline;
|
||||
- whether the ticket reproduced (the SHA if it did not);
|
||||
- the rival and where it failed;
|
||||
- what you could not verify, and why;
|
||||
- proposed repo-map and `kb.pl` text;
|
||||
- **what the ticket did not predict**, consistently the most valuable part.
|
||||
|
||||
## Disk
|
||||
|
||||
Each worktree's `target/` grows to 3–6G, and a full disk (ENOSPC) once stopped
|
||||
every agent. **When you hand back, delete your own build output, and only
|
||||
yours.**
|
||||
|
||||
Run `pwd` and `git rev-parse --show-toplevel` first, and read the answer. Then
|
||||
delete by **literal absolute path**, typed out:
|
||||
`rm -rf /Users/.../.claude/worktrees/agent-<you>/target`.
|
||||
|
||||
**Never interpolate a computed path into `rm -rf`.** One agent's worktree had
|
||||
been removed, so `$(git rev-parse --show-toplevel)` printed nothing and the
|
||||
command became `rm -rf "/target"`. A guarded one-liner was tried, but the
|
||||
worktree shell guard refuses it as too complex, which pushed agents back to an
|
||||
unguarded form. A literal path is the version that is both allowed and safe.
|
||||
|
||||
Keep your branch and any uncommitted work. Delete only `target/` (and
|
||||
`harness/opfs-contract/target` if you built it). If you see ENOSPC, stop and
|
||||
report rather than deleting anything outside your worktree.
|
||||
Symlink
+1
@@ -0,0 +1 @@
|
||||
../../.agents/skills/wave
|
||||
@@ -1,83 +0,0 @@
|
||||
---
|
||||
name: wave
|
||||
description: Dispatch parallel agents to work through open beads and shepherd each to a merged PR. Use when an orchestrator is running a wave, partitioning the backlog, or writing a brief for a work-unit agent.
|
||||
---
|
||||
|
||||
# Running a wave
|
||||
|
||||
An orchestrator chunks the open backlog, fans agents out, reviews what comes
|
||||
back, and merges. `AGENTS.md` has the history; this is the operating procedure.
|
||||
|
||||
## Every agent gets its own worktree. No exceptions.
|
||||
|
||||
Dispatch with `isolation: "worktree"`. Without it every agent edits the **same**
|
||||
working tree: one `git status` shows four agents' half-finished files, any commit
|
||||
sweeps up the others' work, and any `git checkout` destroys it. This has happened
|
||||
here. If you discover it mid-wave, stop every agent, have each create a worktree
|
||||
and copy across **only files it can prove it touched**, then `git checkout --`
|
||||
those files in the primary tree.
|
||||
|
||||
In the worktree: `direnv allow`, or `bd` reports a version *mismatch* rather than
|
||||
failing cleanly. `direnv status` prints `Found RC allowed 0`, and `0` means
|
||||
allowed — it reads like the opposite.
|
||||
|
||||
## Partitioning
|
||||
|
||||
By **crate or directory**, never by ticket count. **Tickets that share a file go
|
||||
to one agent**, to be done sequentially — four widget migrations all touching
|
||||
`src/draw.rs` are a guaranteed conflict split four ways. Group first, then count.
|
||||
|
||||
Tell each agent what ground is *not* theirs, and to report rather than guess when
|
||||
it must cross. Two agents adding a workspace member will still collide in
|
||||
`Cargo.toml`/`Cargo.lock`; keep both sides.
|
||||
|
||||
## What every brief must carry
|
||||
|
||||
1. **The expected base commit, by name.** Worktrees do not reliably branch from
|
||||
HEAD — one was created three merges behind and only noticed because it was
|
||||
told to check.
|
||||
2. **The baseline**, so "N passing" is checkable rather than asserted: Rust test
|
||||
count, harness counts, node counts.
|
||||
3. **Its ground, and the ground that is not.**
|
||||
4. **Never write** `.beads/**`, `prolog-sessions/kb.pl`, `context/repo-map.md`.
|
||||
Agents read them freely and report proposed text; the orchestrator applies it.
|
||||
5. **The commit trailers and PR footer**, spelled out.
|
||||
6. **Do not push to `main`, do not merge.** The orchestrator merges.
|
||||
7. **What to report**: branch and head SHA, PR number, what changed per ticket,
|
||||
*what could not be verified and why*, proposed repo-map/kb text, and **what it
|
||||
learned that the ticket did not predict** — consistently the most valuable
|
||||
part, and omitted unless asked for explicitly.
|
||||
|
||||
## Method to require, because this repo's defect is a test that passes
|
||||
|
||||
- Failing test first, then the fix.
|
||||
- Every guard proved by **injecting the fault it names** and watching it fail
|
||||
**by name**. An injection that does not compile reads as a clean run, so
|
||||
require a positive signal the build ran.
|
||||
- `cargo test --workspace` is **fail-fast**; `--no-fail-fast` for any claim about
|
||||
blast radius.
|
||||
- A textually clean merge can **delete tests** with no conflict marker, and
|
||||
`cargo check` will not notice. After any merge that moves a test module, diff
|
||||
test **names** — a count is masked by the other side's additions.
|
||||
|
||||
## Shepherding
|
||||
|
||||
- Wait on checks with `scripts/pr-watch <pr> --expect <sha>`. Never hand-roll a
|
||||
poll; see the `pr-watch` skill for the three ways they report green while wrong.
|
||||
- Review before merging. Copilot's quota may be exhausted and its request is a
|
||||
**silent no-op**; an absent review and an unrequested one both read as
|
||||
"pending". Dispatch an adversarial reviewer and give it the head SHA, the base,
|
||||
and a test-merge, so it reviews the tree that will exist.
|
||||
- A re-review is of the **delta**, at a named SHA. A review of a superseded
|
||||
commit reads as a pass.
|
||||
- Merge on GitHub, then `git fetch gh && git merge --ff-only gh/main`.
|
||||
- Anything posted to GitHub goes out under the owner's account: open every
|
||||
comment with a line saying a Claude agent wrote it.
|
||||
|
||||
## Between waves
|
||||
|
||||
Run the code-review skill on the merged range. The characteristic find here is
|
||||
not a wrong line, it is **a test that passes while the thing it names is
|
||||
broken** — a vacuous fold test, a one-category fixture, a helper that fabricates
|
||||
what the code under test needs. Ask the reviewer to inject the fault and watch
|
||||
the check fail.
|
||||
@@ -100,52 +100,19 @@ Two agents each adding a workspace member will still collide in
|
||||
|
||||
### Every wave agent's brief must include
|
||||
|
||||
1. **Verify your base commit first.** Worktrees do **not** reliably branch from
|
||||
the orchestrator's HEAD — one was created three merges behind and only
|
||||
noticed because it was told to look. `git log --oneline -1` against a named
|
||||
commit, reset onto `main` if behind. Name the expected commit in the brief.
|
||||
2. **The baseline test count**, so "1090 passing" is checkable rather than
|
||||
asserted, and a drop is visible. Tell the agent to **measure its own** and
|
||||
cite the machine: the same commit measured `improvise-io`'s binary at 1.4s on
|
||||
one machine and 3.7–5.2s on another, so a figure with no machine is a claim
|
||||
about the suite, which it is not.
|
||||
3. **The gate list, and it must not say `cargo fmt --all`.** That reaches only
|
||||
**one** of this repo's two workspace roots — `harness/opfs-contract` declares
|
||||
its own `[workspace]` — and the obvious repair is worse: `cargo fmt
|
||||
-p opfs-contract` from the repo root exits **1**, *"not a member of the
|
||||
workspace"*, which is indistinguishable from a real formatting failure
|
||||
(improvise-b4ks). Both spellings fail in the direction that looks like
|
||||
success. The gate is:
|
||||
The wave skill holds both halves, each in one place (improvise-1f8e):
|
||||
|
||||
```
|
||||
direnv exec . ./scripts/fmt-workspaces.sh # exit 2 = the gate did not run
|
||||
```
|
||||
- **The brief checklist** — what an orchestrator must put in a brief — is
|
||||
`.agents/skills/wave/SKILL.md`, "What every brief must carry".
|
||||
- **The agent's standing rules** — base and push, gates, claim, vacuity shapes,
|
||||
evidence, forbidden files, GitHub, hand-back, disk, commit early — are
|
||||
`.agents/skills/wave/agent-rules.md`. A brief points there rather than
|
||||
restating them.
|
||||
|
||||
Every gate goes through `direnv exec .` — `./scripts/checkers.sh` scrubs the
|
||||
environment to an allowlist, so a bare invocation drops `tpl` off `PATH` and
|
||||
reports spurious failures. And **never pipe a gate**: `$?` must be the gate's
|
||||
own, and `check-comment-runs.py --check-evidence` prints two lines with the
|
||||
comment cap **first**, so `| tail -1` silently drops half of it.
|
||||
4. **Commit frequently, so the work is safe in git.** A WIP commit after the
|
||||
first green test, and again after every meaningful step. A worktree is not
|
||||
durable storage: an agent finished a whole ticket with every gate green and
|
||||
lost all of it, uncommitted, when its worktree was deleted during a disk
|
||||
cleanup. Tidying the history before the PR is fine; losing it is not.
|
||||
5. **Do not merge to `main`.** The orchestrator merges. Pushing the agent's own
|
||||
branch (`git push gh HEAD:refs/heads/<branch>`) is encouraged once it is
|
||||
worth keeping.
|
||||
6. The commit-message trailers, spelled out.
|
||||
7. **What to report**: branch and commits, what changed per ticket, *what could
|
||||
not be verified and why*, and proposed repo-map/`kb.pl` text. Ask explicitly
|
||||
for the thing the agent learned that the ticket did not predict — that is
|
||||
consistently the most valuable part of a hand-back, and agents omit it unless
|
||||
asked.
|
||||
|
||||
`bd` works in any worktree, but `.envrc` must be allowed there or you get a
|
||||
**version mismatch** rather than a clean failure — `direnv allow` in the
|
||||
worktree, or rely on the `[whitelist] prefix` in `~/.config/direnv/direnv.toml`.
|
||||
Note `direnv status` reports `Found RC allowed 0`, and `0` means **allowed**
|
||||
(the enum is 0=Allowed, 1=NotAllowed). It reads like the opposite.
|
||||
`.claude/skills/wave` is a symlink to `.agents/skills/wave`, so Claude and Codex
|
||||
read the same files. A new rule goes into one of those two files, not here and
|
||||
not into the next brief: a restated rule is the copy nobody corrects, and this
|
||||
list was once a third copy that drifted from the skill's.
|
||||
|
||||
### Between waves
|
||||
|
||||
|
||||
@@ -1491,6 +1491,19 @@ what says it is closed**. That is the shape `col_groups` already uses for "this
|
||||
grid has no column groups" — an absent list rather than a flag the page has to
|
||||
interpret alongside a mode it would then have to understand.
|
||||
|
||||
**That shape is only sound where "open" implies "non-empty"**, and help is the
|
||||
one place it does — its content is static. `views` and `formulas` borrowed it
|
||||
and were wrong: a document with no formulas opened a formula panel that built
|
||||
the same `RenderModel` as a closed one, so the page hid it under a mode reading
|
||||
FORMULAS (improvise-df2w). Both are now `Option<Vec<_>>` — `None` closed,
|
||||
`Some(vec![])` open and empty — one field, so the two facts cannot disagree,
|
||||
where a `formula_panel_open` flag beside the rows (the `category_panel_open`
|
||||
shape) would have been two. The same mistake as improvise-xg1z: an absence
|
||||
spent on encoding a second fact. What the page does **not** yet draw is the
|
||||
TUI's empty-state line (`(no formulas — press 'n' to add)`); that string lives
|
||||
in `src/ui/formula_panel.rs`, and copying it into `render.rs` would be a second
|
||||
home for it.
|
||||
|
||||
The rows themselves are `improvise_protocol::help::pages()`, the same ones
|
||||
`src/ui/help.rs` draws. `RenderHelpRow` carries a `kind` string
|
||||
(`heading`, `keys`, `accent`, …) which is a *semantic* name the page maps to a
|
||||
|
||||
+113
-10
@@ -218,14 +218,86 @@ its verdict. The list is checked in **both** directions, so an entry that starts
|
||||
being composed fails just as loudly as one that stops
|
||||
(`doctrine(a_stale_list_of_what_is_missing_is_worse_than_no_list)`).
|
||||
|
||||
Two limits are worth stating, because the check is a source scan.
|
||||
`#[cfg(test)]` items are dropped by a line scan that relies on rustfmt putting
|
||||
an item's closing brace at the attribute's own indentation; over-stripping would
|
||||
**fail** the test rather than pass it, which is the safe direction. And a
|
||||
`Command::X` inside `matches!` or `if let` is read as a construction rather than
|
||||
a pattern, which can only make the check *weaker* for that variant — never raise
|
||||
a false alarm. A variant nothing composes at all is immune to both, since its
|
||||
only pattern occurrence is its own fold arm, which `=>` identifies.
|
||||
The check is a source scan, and **what counts as source has one home**:
|
||||
`improvise_protocol::source_scan::production_code`, shared with the forward
|
||||
guard below (improvise-c7mu). It removes three things a scan must not see.
|
||||
|
||||
- **Comments and literals.** Both are blanked, and newlines are kept. Before
|
||||
this, a `// TODO: Command::SetYanked(...)` left where the real composer had
|
||||
been deleted still passed all four tests. That is the *weaker* direction, the
|
||||
one a scan weakness was supposed to be unable to go. It was live in a small
|
||||
way: a `///` in `project.rs` named `Command::SwitchView`, which has real
|
||||
composers, so no verdict was wrong yet. It also had a mirror: a comment
|
||||
explaining that a `NOT_COMPOSED` variant is held back would have raised a
|
||||
false "stale entry" alarm.
|
||||
- **Test items inside a file.** A line scan that relies on rustfmt putting an
|
||||
item's closing brace at the attribute's own indentation. It treats
|
||||
`cfg(any(test, …))` as test code and `cfg(not(test))` as production.
|
||||
Over-stripping **fails** the test rather than passing it, which is the safe
|
||||
direction.
|
||||
- **Test-only files.** A `#[cfg(test)] mod x;` makes `x.rs` and everything under
|
||||
`x/` test code, even though nothing inside the file says so. Before
|
||||
improvise-c7mu, this checker's own file and `improvise-io`'s
|
||||
`roundtrip_tests.rs` were scanned as production.
|
||||
|
||||
One limit remains. A `Command::X` inside `matches!` or `if let` is read as a
|
||||
construction rather than a pattern. That can only make the check *weaker* for
|
||||
that variant; it never raises a false alarm. A variant nothing composes at all
|
||||
is immune, since its only pattern occurrence is its own fold arm, which `=>`
|
||||
identifies.
|
||||
|
||||
### And forwards: every effect is on the wire or has a verdict (improvise-h3y7)
|
||||
|
||||
`vocabulary_is_composed.rs` checks that the vocabulary does not name more than
|
||||
production composes. The other direction is *does the vocabulary name
|
||||
everything it should?* That was the direction `ViewBack` and `ViewForward`
|
||||
failed: they were missing, and no client had a back history. Until improvise-h3y7
|
||||
nothing checked it, and the audit that finally ran it by hand under-reported.
|
||||
Brace-matching `impl Effect for` blocks misses the eight `simple_effect!`
|
||||
expansions and `SetTransientPrefix` in `command/keymap.rs`. All nine are
|
||||
`ViewOnly`, which is the half the `ViewBack` bug was in.
|
||||
|
||||
`improvise-command/src/ui/reduce/vocabulary_is_complete.rs` is that direction.
|
||||
It scans the crate's `src/` for both ways an effect is declared, reading
|
||||
each name past any attributes and doc comments before it. The macro accepts
|
||||
those, and missing them hid effects (improvise-94x2). It requires
|
||||
each name to be a `Command` or `ModelCommand` variant (read off serde's derive)
|
||||
or to appear in `NOT_ON_THE_WIRE` with a verdict. A verdict is one of four
|
||||
kinds, and the check verifies each kind:
|
||||
|
||||
- `Command(v)`: on the wire under another name (`HelpPageNext`/`Prev`/`Set` →
|
||||
`Command::HelpPage`). `v` must be a real variant.
|
||||
- `Frame(v)`: answered by a `ClientFrame` that is not a `Command` (`Save` →
|
||||
`ClientFrame::Save`, gotcha 45). `v` must be a real frame.
|
||||
- `Off(why)`: excluded on purpose, with the argument: session-local residue
|
||||
(gotcha 30), host-local caches, or batch-relative control flow.
|
||||
- `Gap(ticket)`: missing, and a ticket owns closing it (`SaveAs`, `ExportCsv` →
|
||||
improvise-2mqz).
|
||||
|
||||
The list is also checked in reverse. An entry that is now on the wire fails,
|
||||
and so does an entry that names no effect. The scan is `production_code`, so
|
||||
the comments, strings and test fakes that name an effect do not count as
|
||||
effects. If they did, a stale verdict could outlive the effect it names.
|
||||
|
||||
What it does **not** check is whether a same-named pair *agrees*: whether
|
||||
`reduce_view`'s arm does what the effect does. The name match is the
|
||||
completeness half. The semantic half stays a per-pair read (improvise-h3y7's
|
||||
notes put it at about 21 pairs). The resolver lists in `resolve.rs`
|
||||
(`declined()` and the host-only names) are also not checked against the
|
||||
vocabulary yet. The audit found them stale in three places.
|
||||
|
||||
### No `ModelCommand` field is a position (improvise-hkbc)
|
||||
|
||||
improvise-6nsd closed "a stale layout tier misdirects a mutation" on one
|
||||
condition: no `ModelCommand` carries a row or column index. `position_free`
|
||||
in `reduce.rs`'s tests enforces that at compile time. It destructures every
|
||||
variant with no `..`, down into `RecordCell`, and hands each field to a
|
||||
function that accepts only a name, a path, a surrogate, a value or an axis. So
|
||||
a new variant, a new field, or a field retyped to `usize` fails to build, and
|
||||
the build failure forces the ADR conversation. It **cannot** see a position
|
||||
spelled as a `String`. It also does not make the client position-free:
|
||||
improvise-2wu3 resolves a painted `(row, col)` against the current cache before
|
||||
it ever builds a command.
|
||||
|
||||
### Why two model effects are key-path only
|
||||
|
||||
@@ -552,8 +624,39 @@ client over the same grid, and they disagreed by a comma.
|
||||
|
||||
`numeric` and `error` are decided by the server for the same reason: it is
|
||||
the side that knows which it was, and re-deriving client-side is how the two
|
||||
ends drift apart. `CellDisplay` is deliberately not a `CellValue` — by the
|
||||
time a cell reaches a client the number formatting has already happened.
|
||||
ends drift apart. `CellDisplay` is deliberately not *drawn from* a `CellValue`
|
||||
— by the time a cell reaches a client the number formatting has already
|
||||
happened, and `text` is what a renderer paints.
|
||||
|
||||
### `CellDisplay::value` — what `yank` copies (improvise-dsxq)
|
||||
|
||||
The value the host evaluated at this position, the same `Option<CellValue>`
|
||||
that `text` was formatted from: `evaluate_aggregated` over the view's
|
||||
`none_cats` in a pivot grid, `records_value` in a records grid. It exists
|
||||
because the browser's `yy` has to copy what the TUI's `YankCell` copies, and
|
||||
neither string on the cell can be turned back into it:
|
||||
|
||||
- `edit_text` is `CellValue::to_string()`, which writes a non-whole number
|
||||
`{:.4}` — `1.23456789` becomes `1.2346`.
|
||||
- Re-parsing any text with `typed()` turns number-like text into a number —
|
||||
`Text("007")` becomes `Number(7.0)`.
|
||||
- In a pivot grid `edit_text` is the *stored* cell (`get_cell`), so an
|
||||
aggregate or a formula cell has none and yanked nothing at all.
|
||||
|
||||
So the value is **projected**, not re-derived (gotcha 47): a client that
|
||||
wants the value asks for the value. It is kept separate from `edit_text`
|
||||
rather than replacing it, because the two answer different questions — what
|
||||
an editor starts with is a *string*, deliberately the one the TUI's buffer
|
||||
holds, and changing it would change what the TUI's own editor shows.
|
||||
|
||||
It cost `Eq`: `CellValue` holds an `f64`, so `CellDisplay`, `GridCache` and
|
||||
`GridUpdate` are `PartialEq` only. Nothing hashed or `Eq`-bounded them.
|
||||
|
||||
In a records grid the TUI does **not** yank this value: `YankCell` and
|
||||
`PasteCell` resolve the synthetic `(_Index, _Dim)` key, so the TUI yanks
|
||||
`None` and pastes a junk cell into the document (improvise-6p8q). The browser
|
||||
is right there and the TUI is wrong, which is why the lockstep yank test stays
|
||||
on a pivot grid.
|
||||
|
||||
### `AxisFrame` — the grid's frame, and why it is *one* type
|
||||
|
||||
|
||||
@@ -1130,8 +1130,8 @@ Normal and never closes the panel, exactly as `src/draw.rs` keeps drawing it
|
||||
off `view_panel_open` alone. Asserting it hidden would have pinned the browser
|
||||
to behaviour the TUI does not have.
|
||||
|
||||
The gating rule itself — an empty list is what says a panel is closed, the same
|
||||
rule `help` follows — is asserted in Rust as well, by
|
||||
The gating rule itself — `None` is what says a panel is closed (it was an empty
|
||||
list, the rule `help` follows, until improvise-df2w) — is asserted in Rust as well, by
|
||||
`a_closed_panel_projects_no_rows_and_an_open_one_does` and
|
||||
`the_tile_bar_is_always_present_and_only_its_cursor_is_modal`, because a browser
|
||||
test cannot distinguish "closed" from "the cache is empty" and those two tests
|
||||
@@ -4148,6 +4148,12 @@ so the paste is evidence about the yank rather than about a hand-seeded field.
|
||||
|
||||
**Fault injected:** the `"yank"` arm of `composed` renamed. Fails on the yank.
|
||||
|
||||
**Superseded in part by improvise-dsxq** (entry at the end of this file). The
|
||||
fixture's `1200` is the one kind of value on which the edit text and the value
|
||||
agree, so this test could not see that the arm yanked the edit text re-parsed.
|
||||
The arm now yanks `CellDisplay::value`, and the test sets that field on its
|
||||
hand-built cell. The parity claim lives in the lockstep tests, not here.
|
||||
|
||||
### `a_second_key_that_matches_nothing_still_spends_the_prefix`
|
||||
|
||||
`App::dispatch_key` `take()`s the prefix before looking the second key up, so
|
||||
@@ -4225,6 +4231,33 @@ and converged.
|
||||
**Fault injected:** the same `Binding::Prefix` → `Upstream` reversion. Fails on
|
||||
the second press.
|
||||
|
||||
### `an_open_empty_panel_is_not_a_closed_one`
|
||||
|
||||
improvise-df2w. `views` and `formulas` said "closed" by being empty, so an open
|
||||
formula panel over a document with no formulas built a `RenderModel` identical
|
||||
to a closed one, and the page hid it while the mode read FORMULAS. The test
|
||||
compares the two models whole rather than asserting a field, so it does not
|
||||
depend on how the difference is spelled.
|
||||
|
||||
**Reproduced:** fails at HEAD before the fix with `left == right` on the formula
|
||||
half — the two models were byte-identical. The existing
|
||||
`a_closed_panel_projects_no_rows_and_an_open_one_does` could not see it, because
|
||||
it fills both caches before opening the panels.
|
||||
|
||||
### the formula panel opens over a document with no formulas
|
||||
|
||||
The browser half of improvise-df2w, in `harness/tests/contract.spec.js`, over
|
||||
`harness/fixtures/tall.improv`, which has no `## Formulas` section. It asserts
|
||||
**both** directions, because each rival fails a different one.
|
||||
|
||||
**Reproduced:** against a bundle built from the unfixed `render.rs` and
|
||||
`app.js`, fails at `#formulas` `toBeVisible()`, immediately after `#mode` reads
|
||||
FORMULAS — the exact disagreement the ticket describes.
|
||||
|
||||
**Fault injected:** `paintPanel` writing `host.hidden = false` (a page that
|
||||
stops inferring closed and simply never hides). Fails at the launch-time
|
||||
`toBeHidden()` before the panel is opened.
|
||||
|
||||
## improvise-ktlk — the overlay and the commit read one buffer
|
||||
|
||||
PR #162 (improvise-xg1z) changed what the overlay **draws** for an `Editing`
|
||||
@@ -4261,3 +4294,62 @@ with no `SetBuffer`, then presses the keys: Enter must compose
|
||||
- The over-broad seed, "absent **or empty**, refill". This test passes, and
|
||||
`an_edit_buffer_the_user_has_emptied_is_not_refilled_from_the_cell` fails,
|
||||
as it did against the same rival in `render.rs`.
|
||||
|
||||
## improvise-dsxq — the browser's `yy` copies the value, not a string
|
||||
|
||||
PR #161 (improvise-lut7) made the browser's `yank` arm compose
|
||||
`SetYanked(typed(edit_text))`. The TUI's `YankCell` yanks
|
||||
`evaluate_aggregated(key)`, the value itself. Those differ in three ways, and
|
||||
#161's only fixture, `1200`, hits none of them:
|
||||
|
||||
- `edit_text` is `CellValue::to_string()`, which rounds a non-whole number to
|
||||
four places.
|
||||
- `typed()` turns number-like text into a number.
|
||||
- In a pivot grid `edit_text` is the stored cell only, so an aggregate or a
|
||||
formula cell has none and the browser yanked `None`.
|
||||
|
||||
The fix projects the evaluated value as `CellDisplay::value`, and the arm yanks
|
||||
that (`context/code/protocol.md` §3). Reproduced at `40b8913` by both tests
|
||||
below before the fix: the browser yanked `Number(1.2346)` for `1.23456789`, and
|
||||
`None` for the aggregate.
|
||||
|
||||
### `yy_then_p_copies_the_value_the_host_copies`
|
||||
|
||||
Three cases, one per disagreement, each on a real `App` and a `ClientState`
|
||||
seeded by `grid_of`. The host presses `y y p` through `reduce_key`; a twin
|
||||
host applies exactly what the client composed. Each case asserts the host
|
||||
yanked the expected value (so a fixture that yanks nothing cannot pass), that
|
||||
the client yanked the same, that the host's paste landed, and that the two
|
||||
documents match. Sources sit at row 2 or 3 and targets at row 1, never row 0,
|
||||
and the first column is an unused `Units` measure, so no cursor is at index 0.
|
||||
|
||||
- `1.23456789`, a non-whole number.
|
||||
- `Text("007")`, number-like text.
|
||||
- `Profit = Revenue - Cost` over `10.123456` and `0.5`, a formula's value,
|
||||
pasted into a stored `Revenue` cell.
|
||||
|
||||
**Faults injected, each run:**
|
||||
- The pre-fix arm, `typed(edit_text)`. Fails on the first case with
|
||||
`Number(1.2346)`.
|
||||
- The arm yanking `typed(&value.to_string())`, the projected value re-rendered.
|
||||
Fails on the first case with `Number(1.2346)`.
|
||||
- The producer coercing number-like `Text` through `typed()`. The first case
|
||||
passes, and the second fails with `Number(7.0)`.
|
||||
- The producer projecting the raw `get_cell` instead of the evaluated value.
|
||||
The first two cases pass, and the third fails with `None`.
|
||||
|
||||
### `yy_over_an_aggregate_copies_the_sum_the_host_copies`
|
||||
|
||||
`Product` on the None axis with two stored records, `1.23456789` and `1.0`, so
|
||||
the cell at row 1 is an `Aggregate` with no stored value. Yank only, since an
|
||||
all-aggregate grid has no record cell to paste into. The host must yank
|
||||
`Number(2.23456789)`, which rules out a fixture that is not an aggregate.
|
||||
|
||||
**Faults injected, each run:** the pre-fix arm (fails with `None`), the
|
||||
re-rendered value (fails with `Number(2.2346)`), and the raw `get_cell`
|
||||
producer (fails with `None`).
|
||||
|
||||
Records mode is deliberately not covered. There the TUI yanks `None` and its
|
||||
`p` writes a junk `_Dim`/`_Index` cell, because it resolves the synthetic key
|
||||
(improvise-6p8q). The browser is right and the TUI is wrong, so a lockstep
|
||||
case there would assert the bug.
|
||||
|
||||
@@ -2382,6 +2382,31 @@ keystroke to land in.
|
||||
Injected fault: dropping `self.view_state = snapshot.view_state` from
|
||||
`App::restore` fails this test and only this test.
|
||||
|
||||
**Addendum (improvise-gho5).** Improvise-gho5 found that this
|
||||
test and `a_rolled_back_load_tells_nobody_and_puts_every_mirror_back` passed
|
||||
against a host that **never ran the load**. The review's fault, reproduced
|
||||
here, was placed at the top of `Host::commands`: refuse any multi-command frame
|
||||
that contains a `LoadModel` with `Rejected(DocumentReplaced)`, before
|
||||
`CheckedBatch` runs. Both tests stayed green. A frame that never got that far
|
||||
leaves the generation, the view, the status line and every mirror untouched,
|
||||
which is exactly what the tests asserted.
|
||||
|
||||
Both tests now assert that the asker's whole reply is
|
||||
`the_load_ran_then_the_write_broke()`, which is `Rejected(ChainBroken(North))`.
|
||||
They also assert, before the frame, that `North` resolves. Together these make
|
||||
the rejection a witness that the load ran. `ChainBroken` comes only from
|
||||
`reduce::broken_chain`, which runs inside `mutating_frame` just before each
|
||||
mutation. It names `North` only once the document no longer holds that
|
||||
record, and only an applied `LoadModel` in this frame can have removed it.
|
||||
|
||||
With the same fault re-injected, both tests fail. The left side is
|
||||
`[Rejected(DocumentReplaced)]` and the right is
|
||||
`[Rejected(ChainBroken(<North>))]`. The fault was reverted.
|
||||
|
||||
The residual: a rogue up-front check that forged `ChainBroken` specifically
|
||||
would still pass. That is the same forgery a hand-rolled client could commit
|
||||
against any basis check, and a test cannot guard against it.
|
||||
|
||||
### a_refused_mutation_rolls_back_its_predecessor_and_the_abort_flag
|
||||
|
||||
The `Rejection::MutationRefused` arm — the one the first hand-back wrongly called
|
||||
@@ -4885,9 +4910,14 @@ Three things make it non-vacuous:
|
||||
`LoadModel` silently failed, `Applied::Rebased` would never run and every
|
||||
assertion here would pass over nothing. The control asserts the generation
|
||||
moves and that *both* clients are owed a `Snapshot`.
|
||||
- **The rollback is asserted before anything is concluded from it** — the
|
||||
`document_generation` equality is checked first, so "the frame was rejected"
|
||||
cannot be confused with "the frame never got that far".
|
||||
- **The load is witnessed to have RUN before anything is concluded from the
|
||||
rollback** (improvise-gho5). The asker's whole reply must be
|
||||
`Rejected(ChainBroken(North))`. `North` is asserted to resolve before the
|
||||
frame, so only `broken_chain` can give that answer, and only after the
|
||||
`LoadModel` has replaced the document. `CheckedBatch` never produces
|
||||
`ChainBroken`. This bullet used to say the `document_generation` equality
|
||||
did the job, and that was false: generation equality is exactly what holds
|
||||
when the frame never got that far.
|
||||
- **A bystander is seated where a wrong relay could move it.** A single-client
|
||||
fixture cannot tell "the mirrors were restored" from "there was one mirror and
|
||||
it happened to be the asker's".
|
||||
@@ -4903,3 +4933,42 @@ The control for the test above, and it is stated separately rather than folded
|
||||
in so that a change making the fixture's load stop rebasing fails *here*, where
|
||||
the message says what actually broke, rather than turning the rollback test
|
||||
green by emptying it.
|
||||
|
||||
### ui::reduce::vocabulary_is_complete::every_effect_is_on_the_wire_or_has_a_verdict
|
||||
|
||||
improvise-h3y7, the forward direction of the vocabulary derivation. It is the
|
||||
direction `ViewBack`/`ViewForward` failed: they were missing, so no client ever
|
||||
had a back history. The test scans this crate's production `src/` for
|
||||
`impl Effect for X` and `simple_effect!(X`. That covers both spellings, and the
|
||||
h3y7 audit's brace-matcher missed nine effects by seeing only the first. It
|
||||
then requires each name to be a `Command` or `ModelCommand` variant (read off
|
||||
serde's derive) or to carry a `NOT_ON_THE_WIRE` verdict.
|
||||
|
||||
Faults injected, together: `HelpPageNext`'s verdict replaced by one for
|
||||
`SetSelected`. This test failed naming `HelpPageNext`, and
|
||||
`the_verdict_list_names_nothing_that_is_on_the_wire` failed naming
|
||||
`SetSelected`. Separately, `HelpPageSet`'s verdict was pointed at
|
||||
`Command::HelpPag`, and
|
||||
`the_verdict_list_names_only_real_effects_and_real_variants` failed.
|
||||
|
||||
`the_scan_reaches_every_way_an_effect_is_declared` pins one effect of each
|
||||
spelling plus `SetTransientPrefix` (which lives in `command/keymap.rs`). A
|
||||
broken walk therefore fails that test instead of shrinking the set.
|
||||
`an_effect_named_only_in_a_comment_or_string_is_not_an_effect` shares
|
||||
improvise-c7mu's fault. With `code_only` made the identity, it failed with
|
||||
`["SaveAs", "ExportCsv", "Real", "SetYanked", "AlsoReal"]`.
|
||||
|
||||
**improvise-94x2, found by the PR #172 review.** The reader took the
|
||||
identifier *immediately* after `simple_effect!(`. The macro's own matcher is
|
||||
`$(#[$meta:meta])* $name:ident(…)`, so a doc comment before the name (blanked
|
||||
to whitespace by `code_only`), an attribute, or a line break after `(` left
|
||||
nothing to read, and the effect was invisible. The reviewer injected a
|
||||
documented `simple_effect!(\n /// Rival.\n SetBogusRival(bool) …)` into
|
||||
`effect.rs`, and all five tests passed. Re-injected here, it also passed all
|
||||
five before the fix. After the fix
|
||||
`every_effect_is_on_the_wire_or_has_a_verdict` fails naming `SetBogusRival`.
|
||||
The rival was removed afterwards. `name_after` now skips whitespace and every
|
||||
bracket-matched `#[…]` before reading the name.
|
||||
`an_attributed_or_wrapped_effect_is_still_an_effect` pins that on a fixture:
|
||||
a doc comment, two attributes (one with nested brackets), and a wrapped call.
|
||||
With the whitespace skip removed it fails with `[]`.
|
||||
|
||||
@@ -969,3 +969,63 @@ That assertion is the discriminating one: a fold arm that removed the key would
|
||||
give `None` and fail. Asserting only that `"command"` survived would pass under
|
||||
either behaviour, which is what the old spelling amounted to for the cleared
|
||||
key.
|
||||
|
||||
### vocabulary_is_composed::a_variant_named_only_in_a_comment_or_string_is_not_composed
|
||||
|
||||
improvise-c7mu, found by the post-merge review of PR #160. `constructed_in` ran
|
||||
over raw source text, so any `Command::X` counted as a composer, including one
|
||||
inside a comment or a string. The review deleted the real `Command::SetYanked`
|
||||
composer from `resolve.rs`, left `// TODO: Command::SetYanked(value)` in its
|
||||
place, and all four tests passed. The test feeds the whole per-file pipeline a
|
||||
line comment, a block comment and a string that each name a variant, and
|
||||
asserts that none of them counts. A positive control asserts that a real
|
||||
construction still does.
|
||||
|
||||
Fault injected: `code_only` made the identity function. This test failed with
|
||||
`["SetYanked", "SetSearchMode", "ChangeMode"]`, and so did
|
||||
`source_scan::tests::comments_and_literals_are_blanked_and_code_is_kept`. The
|
||||
four whole-tree tests stayed green under that fault. This confirms the review's
|
||||
finding that no live comment gives a wrong verdict *today*, and it is the
|
||||
reason the property has to be pinned on a fixture rather than on the tree.
|
||||
|
||||
The fault on the real tree (deleting `resolve.rs`'s composer) was not rerun
|
||||
here. `resolve.rs` is outside this lane, and the edit was refused as another
|
||||
agent's ground.
|
||||
|
||||
### source_scan::tests::a_test_only_module_file_is_not_production_code
|
||||
|
||||
improvise-c7mu, the second half. `#[cfg(test)] mod x;` makes a whole file test
|
||||
code, and nothing inside the file says so. The old scan saw it as production,
|
||||
and that included this checker's own file. The test builds a fixture tree:
|
||||
`lib.rs` declares `mod p;` and `#[cfg(test)] mod t;`, and `t.rs` declares a
|
||||
nested `mod deeper;`. It asserts that `production_code` returns exactly
|
||||
`lib.rs` and `p.rs`.
|
||||
|
||||
Fault injected: the file filter in `production_code` made always-true. This
|
||||
test failed with `["lib.rs", "p.rs", "t.rs", "t/deeper.rs"]`. Before this
|
||||
fixture existed, the `improvise-protocol` and `improvise-command` lib suites
|
||||
**stayed green** under the same fault, and so did
|
||||
`test_only_items_and_modules_are_not_production`, which checks the helpers one
|
||||
by one. The wiring between them was untested until this fixture existed.
|
||||
|
||||
### reduce::tests::every_model_command_field_is_position_free
|
||||
|
||||
improvise-hkbc. improvise-6nsd's "a stale layout tier cannot misdirect a
|
||||
mutation" holds only while no `ModelCommand` carries a row or column. The guard
|
||||
is the signature of `position_free`. It destructures every variant with no `..`,
|
||||
down into `RecordCell`, and hands each field to a function that accepts only
|
||||
`&str`, a path, a `RecordHash`, a `CellValue`, an `Axis` or an optional name.
|
||||
The `#[test]` only keeps that signature compiled and reached.
|
||||
|
||||
Faults injected, one per half, each compiling only `improvise-protocol`:
|
||||
|
||||
- A new variant `ScrollTo { row: usize }`, also added to `records_named`'s
|
||||
or-pattern the way an author would. The lib built. The test build failed
|
||||
with `non-exhaustive patterns: ScrollTo` **at `position_free` and nowhere
|
||||
else**.
|
||||
- The same variant classified as `ScrollTo { row } => name(row)`. The test
|
||||
build failed with `expected &str, found &usize`.
|
||||
|
||||
A field added to an existing variant is caught too, but not in isolation,
|
||||
because every constructor of that variant also fails. Reverted with
|
||||
`git restore`.
|
||||
|
||||
@@ -391,6 +391,8 @@ green suite over a vacuous assertion.
|
||||
|
||||
**What is not covered.** The script asks `cargo metadata` for members, so a root
|
||||
whose metadata succeeds but whose member list is wrong is not detectable here.
|
||||
*Narrowed by improvise-90ea, below:* a list that **omits** a package in the tree
|
||||
is now refused; one that names a package it should not is still not detected.
|
||||
And `harness/opfs-contract`'s own `cargo fmt -p opfs-contract` step in the
|
||||
`harness-lint` job is left as it is: it resolves correctly because that step
|
||||
sets `working-directory`, and the ticket's own note confirms CI as written was
|
||||
@@ -415,9 +417,11 @@ getting wrong. The line the split actually needs is **"did the gate run"**:
|
||||
- exit 1 — rustfmt did not certify a root clean. A diff or a refusal to parse;
|
||||
either way **a file is named** and the tree is what is wrong.
|
||||
- exit 2 — the gate did not run. Usage, no cargo, no rustfmt, `cargo metadata`
|
||||
failed, no `[workspace]` root. **No file is named.** Every such cause is
|
||||
checked *above* the loop, which is what makes the narrowing true rather than
|
||||
hopeful.
|
||||
failed, no `[workspace]` root. **No source file is named.** Every such cause
|
||||
is checked *above* the loop, which is what makes the narrowing true rather
|
||||
than hopeful. (Since improvise-90ea, a package no root reaches is also exit 2,
|
||||
and it names that package's `Cargo.toml`, which is the reason no verdict
|
||||
exists. The line is "no verdict", not "no output"; improvise-ximf.)
|
||||
|
||||
Both directions are now pinned, and two further rivals were injected:
|
||||
|
||||
@@ -439,6 +443,67 @@ stub must fail in the **shape** of the real fault, not merely fail — a stub th
|
||||
breaks more than the real thing breaks satisfies the test through the wrong arm,
|
||||
and reads exactly like coverage.
|
||||
|
||||
#### The root package, and why the fourth fix is structural — `scripts/fmt-workspaces-test.sh`
|
||||
|
||||
A post-merge review of PR #159 found a **fourth** fixture vacuity
|
||||
(improvise-90ea). The fixture's outer root is a package (`root-pkg`) as well as
|
||||
a workspace, which is right, but no case ever dirtied **the root package's own
|
||||
`src/lib.rs`**. Only the member `a/` and `nested/` were dirtied. So a script
|
||||
that formats every member *except* the root package passed 17/17. In this repo
|
||||
the root package is `improvise`, the whole TUI. Reproduced at `40b8913` with
|
||||
the review's rival in `members_of` (drop the package whose manifest sits at
|
||||
`workspace_root`): the suite passed 17/17 and `--list` left `improvise` out.
|
||||
|
||||
Adding one more hand-picked case would have fixed the fourth and not the fifth.
|
||||
Four vacuities in one checker all have the same cause: **the cases were chosen
|
||||
by hand, one per hazard somebody had thought of.** So the fix is structural,
|
||||
and it has two halves because it has to cover two sets of packages:
|
||||
|
||||
- **Every package the fixture has.** The two hand-picked fault cases are
|
||||
replaced by a loop over every `[package]` manifest in the fixture. Each one
|
||||
gets its own misformatted `src/lib.rs`, must exit 1, and must be named
|
||||
**exactly once**, so the `--all` boundary check now covers every package
|
||||
rather than one. The enumeration is a `grep` for `[package]`, deliberately
|
||||
*not* the script's `[workspace]` + `cargo metadata` discovery, since that
|
||||
discovery is what is under test. A count assertion (three packages) stops
|
||||
the loop going vacuous if the fixture loses a package. A package added to
|
||||
the fixture gets its case automatically.
|
||||
- **Every package the tree has.** The loop proves only that the script handles
|
||||
the fixture's *shapes*. A real tree can hold a package no root lists (an
|
||||
`exclude`d crate, say), and nothing in the fixture would say so. So the gate
|
||||
itself now checks an invariant **before it formats anything**: every
|
||||
`[package]` manifest the find reaches must be a member of some root, or it
|
||||
exits **2** and names the package. That is exit 2 and not 1 because no
|
||||
verdict was produced for that package, which is what exit 2 means here.
|
||||
Paths are compared through `realpath`, because on macOS `mktemp` lives under
|
||||
`/var` while cargo reports `/private/var`.
|
||||
|
||||
**Rivals injected, and where each failed** (the suite went from 17 to 21 cases):
|
||||
|
||||
- **The review's skip-the-root rival.** It now fails the fixture at `a fault in
|
||||
src/lib.rs is reported` (exit 0). With the invariant in place it fails
|
||||
**six** cases (exit 2, "no workspace root reaches"). It also fails the
|
||||
**real repo** with no fixture involved: `fmt-workspaces.sh` exits 2 naming
|
||||
`Cargo.toml`. That last one is the property worth having. The gate refuses
|
||||
its own blindness in CI, not only in its test suite.
|
||||
- **Disabling the invariant** (`if false`). It fails `a package no workspace
|
||||
root reaches is exit 2, not a pass` with **exit 0**, even though that
|
||||
package's `src/lib.rs` is misformatted. That exit 0 *is* the hazard: an
|
||||
unreached package is silently clean.
|
||||
- **The two earlier rivals, re-run against the loop.** `--all` for `-p` fails
|
||||
both `a/src/lib.rs` and `src/lib.rs` "named exactly once". The second of
|
||||
those was not predicted: the nested root's `--all` also reaches the root
|
||||
package. The bare manifest-path form fails `a fault in a/src/lib.rs is
|
||||
reported`.
|
||||
|
||||
The real-repo control from the ticket still holds: a misformatted fn appended
|
||||
to `src/ui/text.rs` is reported (`Diff in …/src/ui/text.rs:72`).
|
||||
|
||||
**What is still not covered.** A root whose member list *includes* something it
|
||||
should not is not detected, and a package in a directory the find excludes
|
||||
(`target/`, `.claude/`, `node_modules/`) is outside the invariant by
|
||||
construction.
|
||||
|
||||
### The kb tool's completeness guard and atomic write — `scripts/kb-test.sh` cases 18–22
|
||||
|
||||
improvise-usus: `scripts/kb add` wrote onto a working-tree `kb.pl` that had lost
|
||||
|
||||
@@ -1049,6 +1049,12 @@ mod tests {
|
||||
let was_view = host.app().view_state.active_view.clone();
|
||||
let was_status = host.app().view_state.status_msg.clone();
|
||||
let basis = frame_basis(&host, &["North"]);
|
||||
assert!(
|
||||
host.app()
|
||||
.resolve_record(addressed("North").record)
|
||||
.is_some(),
|
||||
"North must resolve BEFORE the load, or a refusal naming it proves nothing"
|
||||
);
|
||||
|
||||
// The load lands, then the write names a record the incoming document
|
||||
// has never had — so the frame fails at its second command.
|
||||
@@ -1069,11 +1075,10 @@ mod tests {
|
||||
]),
|
||||
);
|
||||
|
||||
assert!(
|
||||
answer
|
||||
.iter()
|
||||
.any(|a| matches!(a.msg, ServerFrame::Rejected(_))),
|
||||
"the fixture must actually refuse the frame: {answer:?}"
|
||||
assert_eq!(
|
||||
to(&answer, asker),
|
||||
vec![the_load_ran_then_the_write_broke()],
|
||||
"only a load that RAN can refuse the write this way: {answer:?}"
|
||||
);
|
||||
assert_eq!(
|
||||
host.app().view_state.active_view,
|
||||
@@ -1097,6 +1102,12 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// `North` resolves in the outgoing document, so only `broken_chain` AFTER the
|
||||
/// load applied can answer this. Evidence: `context/test-evidence/command.md`.
|
||||
fn the_load_ran_then_the_write_broke() -> ServerFrame {
|
||||
ServerFrame::Rejected(Rejection::ChainBroken(addressed("North").record))
|
||||
}
|
||||
|
||||
/// Everything one client was told, flattened to the projections it carries.
|
||||
fn projections(frames: &[Addressed], id: SessionId) -> Vec<Projection> {
|
||||
to(frames, id)
|
||||
@@ -1147,6 +1158,12 @@ mod tests {
|
||||
let (bystander, _) = host.connect(10, 10);
|
||||
let was_generation = host.app().document_generation();
|
||||
let basis = frame_basis(&host, &["North"]);
|
||||
assert!(
|
||||
host.app()
|
||||
.resolve_record(addressed("North").record)
|
||||
.is_some(),
|
||||
"North must resolve BEFORE the load, or a refusal naming it proves nothing"
|
||||
);
|
||||
|
||||
// The load lands and rebases every mirror; the write then names a
|
||||
// record the incoming document has never had, so the frame fails.
|
||||
@@ -1167,6 +1184,11 @@ mod tests {
|
||||
]),
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
to(&answer, asker),
|
||||
vec![the_load_ran_then_the_write_broke()],
|
||||
"only a load that RAN can refuse the write this way: {answer:?}"
|
||||
);
|
||||
assert_eq!(
|
||||
host.app().document_generation(),
|
||||
was_generation,
|
||||
|
||||
@@ -282,6 +282,7 @@ fn cell_display(
|
||||
numeric: matches!(value, Some(CellValue::Number(_))),
|
||||
error: matches!(value, Some(CellValue::Error(_))),
|
||||
id: cell_id(app, layout, row, col),
|
||||
value: value.clone(),
|
||||
};
|
||||
}
|
||||
let key = layout.cell_key(row, col);
|
||||
@@ -293,6 +294,7 @@ fn cell_display(
|
||||
// The *unformatted* value the TUI seeds its edit buffer from. Projecting it
|
||||
// beats a client un-formatting `text` (gotcha 47).
|
||||
edit_text: key.map(|key| app.display_value_in(layout, &key)),
|
||||
value: value.clone(),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -10,6 +10,9 @@ use crate::ui::app::App;
|
||||
use crate::ui::effect::{self, Effect, ViewTarget};
|
||||
use crate::ui::project::{Dirty, document_projections};
|
||||
|
||||
#[cfg(test)]
|
||||
mod vocabulary_is_complete;
|
||||
|
||||
/// A wire [`Command`] whose basis the authoritative state has answered for
|
||||
/// (improvise-ondj/7v62, ADR 001). See `context/code/command.md`.
|
||||
pub struct Checked<'a>(&'a Command);
|
||||
|
||||
@@ -0,0 +1,247 @@
|
||||
//! Every effect is on the wire under its own name, or named below with its
|
||||
//! verdict: the forward twin of `vocabulary_is_composed.rs`. `context/code/protocol.md` §2.
|
||||
|
||||
use std::collections::BTreeSet;
|
||||
use std::path::Path;
|
||||
|
||||
use improvise_protocol::source_scan::{production_code, production_text};
|
||||
use improvise_protocol::variants::declared;
|
||||
use improvise_protocol::{ClientFrame, Command, ModelCommand};
|
||||
|
||||
/// Why an effect has no same-named `Command`/`ModelCommand` variant.
|
||||
enum Verdict {
|
||||
/// On the wire as this `Command` variant, under another name.
|
||||
Command(&'static str),
|
||||
/// Answered by this `ClientFrame` variant, which is not a `Command`.
|
||||
Frame(&'static str),
|
||||
/// Deliberately off the wire, and the argument.
|
||||
Off(&'static str),
|
||||
/// Missing, and the ticket that owns closing it.
|
||||
Gap(&'static str),
|
||||
}
|
||||
|
||||
/// Checked in all directions: an entry must name a real effect, must not be on
|
||||
/// the wire by name, and a `Command`/`Frame` verdict must name a real variant.
|
||||
const NOT_ON_THE_WIRE: &[(&str, Verdict)] = &[
|
||||
("HelpPageNext", Verdict::Command("HelpPage")),
|
||||
("HelpPagePrev", Verdict::Command("HelpPage")),
|
||||
("HelpPageSet", Verdict::Command("HelpPage")),
|
||||
("Save", Verdict::Frame("Save")),
|
||||
(
|
||||
"SaveAs",
|
||||
Verdict::Gap("improvise-2mqz: writes the host's Storage and has no frame"),
|
||||
),
|
||||
(
|
||||
"ExportCsv",
|
||||
Verdict::Gap("improvise-2mqz: writes the host's Storage and has no frame"),
|
||||
),
|
||||
(
|
||||
"ApplyAndClearDrill",
|
||||
Verdict::Off("operand is one session's staged drill edits (gotcha 38, improvise-3x3u)"),
|
||||
),
|
||||
(
|
||||
"AcceptWizardModel",
|
||||
Verdict::Off("operand is one session's built wizard workbook (gotcha 38, improvise-3x3u)"),
|
||||
),
|
||||
(
|
||||
"StartDrill",
|
||||
Verdict::Off("writes drill_state, session-local residue (gotcha 30)"),
|
||||
),
|
||||
(
|
||||
"SetDrillPendingEdit",
|
||||
Verdict::Off("writes drill_state.pending_edits, session-local residue (gotcha 30)"),
|
||||
),
|
||||
(
|
||||
"StartImportWizard",
|
||||
Verdict::Off("writes App::wizard, session-local residue (gotcha 30, improvise-764)"),
|
||||
),
|
||||
(
|
||||
"WizardApply",
|
||||
Verdict::Off("writes App::wizard, session-local residue (gotcha 30, improvise-764)"),
|
||||
),
|
||||
(
|
||||
"CloseWizard",
|
||||
Verdict::Off("writes App::wizard, session-local residue (gotcha 30, improvise-764)"),
|
||||
),
|
||||
(
|
||||
"RebuildLayout",
|
||||
Verdict::Off(
|
||||
"refreshes the host's layout cache mid-batch; a client holds none (gotcha 23)",
|
||||
),
|
||||
),
|
||||
(
|
||||
"AbortChain",
|
||||
Verdict::Off("control flow inside one apply_effects batch; it writes no state (gotcha 21)"),
|
||||
),
|
||||
(
|
||||
"ReturnToNormalUnlessMoved",
|
||||
Verdict::Off("relative to the batch it ends (gotcha 31); a client composes ChangeMode"),
|
||||
),
|
||||
(
|
||||
"EnterEditAtCursor",
|
||||
Verdict::Off(
|
||||
"reads the host's layout; a client composes SetBuffer + ChangeMode off its own cache",
|
||||
),
|
||||
),
|
||||
];
|
||||
|
||||
#[test]
|
||||
fn every_effect_is_on_the_wire_or_has_a_verdict() {
|
||||
let on_the_wire = wire_names();
|
||||
let missing: Vec<String> = effects_in_production()
|
||||
.into_iter()
|
||||
.filter(|name| !on_the_wire.contains(name))
|
||||
.filter(|name| !NOT_ON_THE_WIRE.iter().any(|(n, _)| n == name))
|
||||
.collect();
|
||||
assert!(
|
||||
missing.is_empty(),
|
||||
"effect::{missing:?} has no Command or ModelCommand of its name and no \
|
||||
verdict. Put it on the wire, or name it in NOT_ON_THE_WIRE with the \
|
||||
argument (improvise-h3y7) — gotcha 38's ViewBack was exactly this"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_verdict_list_names_nothing_that_is_on_the_wire() {
|
||||
let on_the_wire = wire_names();
|
||||
let stale: Vec<&str> = NOT_ON_THE_WIRE
|
||||
.iter()
|
||||
.map(|(name, _)| *name)
|
||||
.filter(|name| on_the_wire.contains(*name))
|
||||
.collect();
|
||||
assert!(
|
||||
stale.is_empty(),
|
||||
"effect::{stale:?} has a verdict and IS on the wire. \
|
||||
doctrine(a_stale_list_of_what_is_missing_is_worse_than_no_list)"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_verdict_list_names_only_real_effects_and_real_variants() {
|
||||
let effects = effects_in_production();
|
||||
let commands = declared::<Command>();
|
||||
let frames = declared::<ClientFrame>();
|
||||
for (name, verdict) in NOT_ON_THE_WIRE {
|
||||
assert!(effects.contains(*name), "no effect is named {name}");
|
||||
match verdict {
|
||||
Verdict::Command(variant) => assert!(
|
||||
commands.iter().any(|c| c == variant),
|
||||
"{name}'s verdict names Command::{variant}, which does not exist"
|
||||
),
|
||||
Verdict::Frame(variant) => assert!(
|
||||
frames.iter().any(|f| f == variant),
|
||||
"{name}'s verdict names ClientFrame::{variant}, which does not exist"
|
||||
),
|
||||
Verdict::Off(why) | Verdict::Gap(why) => {
|
||||
assert!(!why.is_empty(), "{name}'s verdict gives no argument")
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// The scan must reach both spellings of an effect and every file that holds
|
||||
/// one — the h3y7 audit's brace-matcher missed nine that way (check_the_checker).
|
||||
#[test]
|
||||
fn the_scan_reaches_every_way_an_effect_is_declared() {
|
||||
let effects = effects_in_production();
|
||||
for name in ["TransposeAxes", "SetRowOffset", "SetTransientPrefix"] {
|
||||
assert!(
|
||||
effects.contains(name),
|
||||
"the scan missed {name}, so it under-reports: {effects:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn wire_names() -> BTreeSet<String> {
|
||||
declared::<Command>()
|
||||
.into_iter()
|
||||
.chain(declared::<ModelCommand>())
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Every `impl Effect for X` and `simple_effect!(X` in this crate's production
|
||||
/// `src/`: the one scanner both vocabulary guards share (improvise-c7mu).
|
||||
fn effects_in_production() -> BTreeSet<String> {
|
||||
production_code(&[Path::new(env!("CARGO_MANIFEST_DIR")).join("src")])
|
||||
.iter()
|
||||
.flat_map(|(_, code)| effects_named(code))
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// One file's effects. A comment, a string or a test fake naming one is not
|
||||
/// one, or a stale verdict could outlive the effect it names.
|
||||
fn effects_in(text: &str) -> Vec<String> {
|
||||
effects_named(&production_text(text))
|
||||
}
|
||||
|
||||
fn effects_named(code: &str) -> Vec<String> {
|
||||
let mut found = Vec::new();
|
||||
for marker in ["impl Effect for ", "simple_effect!("] {
|
||||
for (at, _) in code.match_indices(marker) {
|
||||
let name = name_after(&code[at + marker.len()..]);
|
||||
if name.starts_with(|c: char| c.is_ascii_uppercase()) {
|
||||
found.push(name);
|
||||
}
|
||||
}
|
||||
}
|
||||
found
|
||||
}
|
||||
|
||||
/// The identifier a marker introduces. `simple_effect!` takes `$(#[$meta])*`
|
||||
/// before the name, and a doc comment is one: skip them (improvise-94x2).
|
||||
fn name_after(rest: &str) -> String {
|
||||
let mut rest = rest.trim_start();
|
||||
while let Some(attr) = rest.strip_prefix("#[") {
|
||||
rest = attr[closing_bracket(attr) + 1..].trim_start();
|
||||
}
|
||||
rest.chars()
|
||||
.take_while(|c| c.is_ascii_alphanumeric() || *c == '_')
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Where the `]` that closes an attribute's `#[` falls in `inside`, the text
|
||||
/// after it. Nested brackets count; an unclosed one runs to the end.
|
||||
fn closing_bracket(inside: &str) -> usize {
|
||||
let mut depth = 1usize;
|
||||
for (i, c) in inside.char_indices() {
|
||||
match c {
|
||||
'[' => depth += 1,
|
||||
']' if depth == 1 => return i,
|
||||
']' => depth -= 1,
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
inside.len().saturating_sub(1)
|
||||
}
|
||||
|
||||
/// The spelling the macro itself documents, a doc comment and an attribute
|
||||
/// before the name, and a line break after `(`. Each hid an effect.
|
||||
#[test]
|
||||
fn an_attributed_or_wrapped_effect_is_still_an_effect() {
|
||||
let text = concat!(
|
||||
"simple_effect!(\n",
|
||||
" /// Documented, as the macro's own matcher allows.\n",
|
||||
" #[allow(dead_code)]\n",
|
||||
" #[cfg_attr(all(), doc = \"x\")]\n",
|
||||
" Documented(bool) => |app| x, K\n",
|
||||
");\n",
|
||||
"simple_effect!(\n Wrapped(bool) => |app| y, K);\n",
|
||||
);
|
||||
assert_eq!(effects_in(text), vec!["Documented", "Wrapped"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_effect_named_only_in_a_comment_or_string_is_not_an_effect() {
|
||||
let text = concat!(
|
||||
"// impl Effect for SaveAs {}\n",
|
||||
"/// simple_effect!(SetYanked(Option<CellValue>) => |app| x, K);\n",
|
||||
"let s = \"impl Effect for ExportCsv\";\n",
|
||||
"impl Effect for Real {}\n",
|
||||
"simple_effect!(AlsoReal(bool) => |app| y, K);\n",
|
||||
"#[cfg(test)]\n",
|
||||
"mod tests {\n",
|
||||
" impl Effect for TestFake {}\n",
|
||||
"}\n",
|
||||
);
|
||||
assert_eq!(effects_in(text), vec!["Real", "AlsoReal"]);
|
||||
}
|
||||
@@ -14,6 +14,8 @@ mod reduce;
|
||||
pub mod render;
|
||||
pub mod resolve;
|
||||
#[cfg(any(test, feature = "test-support"))]
|
||||
pub mod source_scan;
|
||||
#[cfg(any(test, feature = "test-support"))]
|
||||
pub mod variants;
|
||||
pub mod viewmodel;
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -577,4 +577,100 @@ mod tests {
|
||||
assert_eq!(cursor, 7, "{panel:?}'s cursor is not the field it names");
|
||||
}
|
||||
}
|
||||
|
||||
/// improvise-hkbc: every field of every `ModelCommand` names its target by
|
||||
/// name, by path or by surrogate. A row or column index fails to COMPILE.
|
||||
fn position_free(cmd: &ModelCommand) -> usize {
|
||||
fn name(_: &str) -> usize {
|
||||
1
|
||||
}
|
||||
fn surrogate(_: &RecordHash) -> usize {
|
||||
1
|
||||
}
|
||||
fn record(RecordCell { record, measure }: &RecordCell) -> usize {
|
||||
surrogate(record) + optional_name(measure)
|
||||
}
|
||||
fn path(_: &std::path::Path) -> usize {
|
||||
1
|
||||
}
|
||||
fn value(_: &CellValue) -> usize {
|
||||
1
|
||||
}
|
||||
fn axis(_: &Axis) -> usize {
|
||||
1
|
||||
}
|
||||
fn optional_name(_: &Option<String>) -> usize {
|
||||
1
|
||||
}
|
||||
// No `..` anywhere: a field added to a variant must be classified here.
|
||||
match cmd {
|
||||
ModelCommand::SetCell { cell, value: v } => record(cell) + value(v),
|
||||
ModelCommand::ClearCell(cell) => record(cell),
|
||||
ModelCommand::AddItem { category, item }
|
||||
| ModelCommand::RemoveItem { category, item } => name(category) + name(item),
|
||||
ModelCommand::AddItemInGroup {
|
||||
category,
|
||||
item,
|
||||
group,
|
||||
} => name(category) + name(item) + name(group),
|
||||
ModelCommand::AddFormula {
|
||||
raw,
|
||||
target_category,
|
||||
} => name(raw) + name(target_category),
|
||||
ModelCommand::RemoveFormula {
|
||||
target,
|
||||
target_category,
|
||||
} => name(target) + name(target_category),
|
||||
ModelCommand::CleanEmptyRecords | ModelCommand::SortData => 0,
|
||||
ModelCommand::AddCategory(category)
|
||||
| ModelCommand::RemoveCategory(category)
|
||||
| ModelCommand::CreateView(category)
|
||||
| ModelCommand::DeleteView(category) => name(category),
|
||||
ModelCommand::SetAxis {
|
||||
view,
|
||||
category,
|
||||
axis: a,
|
||||
} => name(view) + name(category) + axis(a),
|
||||
ModelCommand::CycleAxis { view, category } => name(view) + name(category),
|
||||
ModelCommand::TransposeAxes { view } | ModelCommand::TogglePruneEmpty { view } => {
|
||||
name(view)
|
||||
}
|
||||
ModelCommand::SetPageSelection {
|
||||
view,
|
||||
category,
|
||||
item,
|
||||
}
|
||||
| ModelCommand::HideItem {
|
||||
view,
|
||||
category,
|
||||
item,
|
||||
}
|
||||
| ModelCommand::ShowItem {
|
||||
view,
|
||||
category,
|
||||
item,
|
||||
} => name(view) + name(category) + name(item),
|
||||
ModelCommand::ToggleGroup {
|
||||
view,
|
||||
category,
|
||||
group,
|
||||
} => name(view) + name(category) + name(group),
|
||||
ModelCommand::SetNumberFormat { view, format } => name(view) + name(format),
|
||||
ModelCommand::LoadModel(p) => path(p),
|
||||
ModelCommand::ImportJsonHeadless {
|
||||
path: p,
|
||||
model_name,
|
||||
array_path,
|
||||
} => path(p) + optional_name(model_name) + optional_name(array_path),
|
||||
}
|
||||
}
|
||||
|
||||
/// The guard is the signature above; this only keeps it compiled and reached.
|
||||
#[test]
|
||||
fn every_model_command_field_is_position_free() {
|
||||
assert_eq!(
|
||||
position_free(&ModelCommand::TransposeAxes { view: "V".into() }),
|
||||
1
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
use serde::{Deserialize, Serialize};
|
||||
use unicode_width::UnicodeWidthStr;
|
||||
|
||||
use improvise_core::model::cell::CellValue;
|
||||
use improvise_core::model::record::{AggregateHash, RecordHash};
|
||||
use improvise_core::view::Axis;
|
||||
use improvise_core::view::layout::PageAxis;
|
||||
@@ -64,9 +65,9 @@ pub enum CreateKind {
|
||||
Category,
|
||||
}
|
||||
|
||||
/// One cell as the client draws it. Deliberately not a `CellValue` — the
|
||||
/// One cell as the client draws it. Drawn from `text`, never from `value` — the
|
||||
/// formatting has already happened, and re-deriving it is how ends drift.
|
||||
#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)]
|
||||
pub struct CellDisplay {
|
||||
pub text: String,
|
||||
/// Right-align numerics, left-align text — decided by the server, which
|
||||
@@ -80,6 +81,9 @@ pub struct CellDisplay {
|
||||
/// What an editor should start with — the value as the user last
|
||||
/// **typed** it, not as it is drawn. The difference corrupts data (§3).
|
||||
pub edit_text: Option<String>,
|
||||
/// The value the host evaluated here, unrounded and untyped-by-text: what
|
||||
/// `yank` copies. Neither `text` nor `edit_text` can be re-parsed into it (§3).
|
||||
pub value: Option<CellValue>,
|
||||
}
|
||||
|
||||
impl CellDisplay {
|
||||
@@ -98,6 +102,7 @@ impl CellDisplay {
|
||||
numeric: true,
|
||||
error: false,
|
||||
id: None,
|
||||
value: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -110,6 +115,7 @@ impl CellDisplay {
|
||||
error: true,
|
||||
id: None,
|
||||
edit_text: None,
|
||||
value: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -416,7 +422,7 @@ fn shows(items: &[&Vec<String>], idx: usize, first_rendered: usize, level: usize
|
||||
|
||||
/// The grid widget's copy of the visible cells (improvise-n10), indexed by
|
||||
/// position within the **viewport** and never by `CellKey`.
|
||||
#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)]
|
||||
pub struct GridCache {
|
||||
/// The row axis's frame: levels, label tuples and groups. **Not** a list
|
||||
/// of strings — see [`AxisFrame`].
|
||||
@@ -563,7 +569,7 @@ enum LabelAxis {
|
||||
|
||||
/// What can change about the grid: cell values, the frames that name them,
|
||||
/// the widths, the totals, the page bar and the extent.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
|
||||
pub enum GridUpdate {
|
||||
/// Display values for cells, by viewport position.
|
||||
Cells(Vec<((usize, usize), CellDisplay)>),
|
||||
|
||||
@@ -530,12 +530,10 @@ fn composed(
|
||||
state,
|
||||
ModelCommand::ClearCell(record_cell_at_cursor(state)?),
|
||||
)?]),
|
||||
// `YankCell::execute`: the value, then the status. `edit_text` is
|
||||
// what the host evaluated, which is what the TUI yanks.
|
||||
// `YankCell::execute`: the value, then the status. The projected
|
||||
// value, never text re-parsed: improvise-dsxq.
|
||||
"yank" => Some(vec![
|
||||
Command::SetYanked(typed(
|
||||
cell_at_cursor(state)?.edit_text.as_deref().unwrap_or(""),
|
||||
)),
|
||||
Command::SetYanked(cell_at_cursor(state)?.value.clone()),
|
||||
Command::SetStatus("Yanked".to_string()),
|
||||
]),
|
||||
"paste" => Some(match state.view.yanked.clone() {
|
||||
@@ -3198,6 +3196,7 @@ mod tests {
|
||||
#[test]
|
||||
fn yy_yanks_the_cell_under_the_cursor_and_p_pastes_what_it_yanked() {
|
||||
let mut state = client_on(record_cell());
|
||||
state.grid.cells[0][0].value = Some(CellValue::Number(1200.0));
|
||||
arm(&mut state, ev(KeyCode::Char('y')));
|
||||
let KeyOutcome::Local(yank) = resolve(&state, ev(KeyCode::Char('y'))) else {
|
||||
panic!("`yy` is bound to yank");
|
||||
|
||||
@@ -0,0 +1,346 @@
|
||||
//! Production Rust source, for the guards that read source text: no comments,
|
||||
//! no literals, no test code. The one home of that rule, `context/code/protocol.md` §2.
|
||||
|
||||
use std::collections::BTreeSet;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
/// Every `.rs` file under `roots` that is compiled outside tests, with its
|
||||
/// comments, literals and test-only items blanked or dropped.
|
||||
pub fn production_code(roots: &[PathBuf]) -> Vec<(PathBuf, String)> {
|
||||
let mut files = Vec::new();
|
||||
for root in roots {
|
||||
collect_rust(root, &mut files);
|
||||
}
|
||||
let code: Vec<(PathBuf, String)> = files
|
||||
.into_iter()
|
||||
.map(|file| {
|
||||
let text = std::fs::read_to_string(&file).expect("a listed source reads");
|
||||
let code = code_only(&text);
|
||||
(file, code)
|
||||
})
|
||||
.collect();
|
||||
let test_modules: BTreeSet<PathBuf> = code
|
||||
.iter()
|
||||
.flat_map(|(file, code)| test_module_paths(file, code))
|
||||
.collect();
|
||||
code.into_iter()
|
||||
.filter(|(file, _)| !test_modules.iter().any(|m| is_module_file(m, file)))
|
||||
.map(|(file, code)| (file, strip_test_items(&code)))
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// One file's text as [`production_code`] cleans it. Which *files* are test
|
||||
/// modules needs their declaring file too, so only `production_code` knows.
|
||||
pub fn production_text(text: &str) -> String {
|
||||
strip_test_items(&code_only(text))
|
||||
}
|
||||
|
||||
fn collect_rust(dir: &Path, into: &mut Vec<PathBuf>) {
|
||||
let Ok(entries) = std::fs::read_dir(dir) else {
|
||||
return;
|
||||
};
|
||||
for entry in entries.flatten() {
|
||||
let path = entry.path();
|
||||
match path.is_dir() {
|
||||
true => collect_rust(&path, into),
|
||||
false if path.extension().is_some_and(|e| e == "rs") => into.push(path),
|
||||
false => {}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// `m` is a module path without extension: its file is `m.rs`, `m/mod.rs`,
|
||||
/// or anything under `m/`.
|
||||
fn is_module_file(m: &Path, file: &Path) -> bool {
|
||||
file == m.with_extension("rs") || file.starts_with(m)
|
||||
}
|
||||
|
||||
/// The out-of-line modules `file` declares under a test-only `cfg`, as paths
|
||||
/// without extension (`#[cfg(test)] mod x;` is `<dir>/x`).
|
||||
fn test_module_paths(file: &Path, code: &str) -> Vec<PathBuf> {
|
||||
let dir = match file.file_stem().and_then(|s| s.to_str()) {
|
||||
Some("lib" | "main" | "mod") => file.parent().map(Path::to_path_buf),
|
||||
Some(stem) => file.parent().map(|p| p.join(stem)),
|
||||
None => None,
|
||||
}
|
||||
.expect("a source file has a directory and a stem");
|
||||
let lines: Vec<&str> = code.lines().map(str::trim).collect();
|
||||
let mut found = Vec::new();
|
||||
for (i, line) in lines.iter().enumerate() {
|
||||
if !is_test_cfg(line) {
|
||||
continue;
|
||||
}
|
||||
let item = lines[i + 1..]
|
||||
.iter()
|
||||
.find(|l| !l.starts_with("#[") && !l.is_empty());
|
||||
if let Some(name) = item.and_then(|l| out_of_line_mod(l)) {
|
||||
found.push(dir.join(name));
|
||||
}
|
||||
}
|
||||
found
|
||||
}
|
||||
|
||||
/// `mod x;`, `pub mod x;`, `pub(crate) mod x;` → `x`.
|
||||
fn out_of_line_mod(line: &str) -> Option<&str> {
|
||||
let rest = line.split_once("mod ")?.1.strip_suffix(';')?;
|
||||
let prefix = line.split_once("mod ")?.0.trim();
|
||||
(prefix.is_empty() || prefix.starts_with("pub")).then_some(rest.trim())
|
||||
}
|
||||
|
||||
/// A `cfg` attribute that compiles its item only under `test`. `cfg(not(test))`
|
||||
/// is production, and `"test-support"` is a literal, already blanked.
|
||||
fn is_test_cfg(line: &str) -> bool {
|
||||
let line = line.trim();
|
||||
line.starts_with("#[cfg(")
|
||||
&& !line.contains("not(")
|
||||
&& line
|
||||
.split(|c: char| !c.is_alphanumeric() && c != '_')
|
||||
.any(|word| word == "test")
|
||||
}
|
||||
|
||||
/// Drop every test-only item. rustfmt puts an item's closing brace at the
|
||||
/// attribute's own indentation, which is what makes this a line scan (§2).
|
||||
fn strip_test_items(text: &str) -> String {
|
||||
let lines: Vec<&str> = text.lines().collect();
|
||||
let mut kept = String::new();
|
||||
let mut i = 0;
|
||||
while i < lines.len() {
|
||||
let line = lines[i];
|
||||
match is_test_cfg(line) {
|
||||
false => {
|
||||
kept.push_str(line);
|
||||
kept.push('\n');
|
||||
i += 1;
|
||||
}
|
||||
true => {
|
||||
let indent = line.len() - line.trim_start().len();
|
||||
let close = format!("{}}}", " ".repeat(indent));
|
||||
i += 1;
|
||||
while i < lines.len() && lines[i].trim_start().starts_with("#[") {
|
||||
i += 1;
|
||||
}
|
||||
match lines.get(i).is_some_and(|l| l.trim_end().ends_with(';')) {
|
||||
true => i += 1,
|
||||
false => {
|
||||
while i < lines.len() && lines[i].trim_end() != close {
|
||||
i += 1;
|
||||
}
|
||||
i += 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
kept
|
||||
}
|
||||
|
||||
/// `text` with comments, string, byte-string, raw-string and char literals
|
||||
/// replaced by spaces. Newlines survive, so line-based scans still line up.
|
||||
pub fn code_only(text: &str) -> String {
|
||||
let chars: Vec<char> = text.chars().collect();
|
||||
let mut out = String::with_capacity(text.len());
|
||||
let mut i = 0;
|
||||
while i < chars.len() {
|
||||
let end = literal_or_comment_end(&chars, i);
|
||||
match end {
|
||||
Some(end) => {
|
||||
for &c in &chars[i..end] {
|
||||
out.push(if c == '\n' { '\n' } else { ' ' });
|
||||
}
|
||||
i = end;
|
||||
}
|
||||
None => {
|
||||
out.push(chars[i]);
|
||||
i += 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
out
|
||||
}
|
||||
|
||||
/// Where the comment or literal starting at `i` ends, if one starts there.
|
||||
fn literal_or_comment_end(c: &[char], i: usize) -> Option<usize> {
|
||||
let at = |k: usize| c.get(k).copied();
|
||||
let word_before = i > 0 && (c[i - 1].is_alphanumeric() || c[i - 1] == '_');
|
||||
match (at(i), at(i + 1)) {
|
||||
(Some('/'), Some('/')) => Some(line_end(c, i)),
|
||||
(Some('/'), Some('*')) => Some(block_end(c, i)),
|
||||
(Some('"'), _) => Some(quoted_end(c, i + 1, '"')),
|
||||
(Some('b'), Some('"')) if !word_before => Some(quoted_end(c, i + 2, '"')),
|
||||
(Some('r' | 'b'), Some('r' | '#' | '"')) if !word_before => raw_end(c, i),
|
||||
(Some('\''), _) => char_end(c, i),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn line_end(c: &[char], from: usize) -> usize {
|
||||
(from..c.len()).find(|&k| c[k] == '\n').unwrap_or(c.len())
|
||||
}
|
||||
|
||||
/// Block comments nest in Rust.
|
||||
fn block_end(c: &[char], from: usize) -> usize {
|
||||
let mut depth = 0usize;
|
||||
let mut k = from;
|
||||
while k + 1 < c.len() {
|
||||
match (c[k], c[k + 1]) {
|
||||
('/', '*') => {
|
||||
depth += 1;
|
||||
k += 2;
|
||||
}
|
||||
('*', '/') => {
|
||||
depth -= 1;
|
||||
k += 2;
|
||||
if depth == 0 {
|
||||
return k;
|
||||
}
|
||||
}
|
||||
_ => k += 1,
|
||||
}
|
||||
}
|
||||
c.len()
|
||||
}
|
||||
|
||||
fn quoted_end(c: &[char], mut k: usize, quote: char) -> usize {
|
||||
while k < c.len() {
|
||||
match c[k] {
|
||||
'\\' => k += 2,
|
||||
q if q == quote => return k + 1,
|
||||
_ => k += 1,
|
||||
}
|
||||
}
|
||||
c.len()
|
||||
}
|
||||
|
||||
/// `r"…"`, `r#"…"#`, `br#"…"#`. `None` when the `r` is only an identifier.
|
||||
fn raw_end(c: &[char], i: usize) -> Option<usize> {
|
||||
let mut k = i + usize::from(c[i] == 'b');
|
||||
if c.get(k) != Some(&'r') {
|
||||
return None;
|
||||
}
|
||||
k += 1;
|
||||
let hashes = c[k..].iter().take_while(|&&h| h == '#').count();
|
||||
k += hashes;
|
||||
if c.get(k) != Some(&'"') {
|
||||
return None;
|
||||
}
|
||||
k += 1;
|
||||
while k < c.len() {
|
||||
if c[k] == '"'
|
||||
&& c[k + 1..]
|
||||
.iter()
|
||||
.take(hashes)
|
||||
.filter(|&&h| h == '#')
|
||||
.count()
|
||||
== hashes
|
||||
{
|
||||
return Some(k + 1 + hashes);
|
||||
}
|
||||
k += 1;
|
||||
}
|
||||
Some(c.len())
|
||||
}
|
||||
|
||||
/// A char literal, or `None` for a lifetime (`'a` with no closing quote).
|
||||
fn char_end(c: &[char], i: usize) -> Option<usize> {
|
||||
match (c.get(i + 1), c.get(i + 2)) {
|
||||
(Some('\\'), _) => Some(quoted_end(c, i + 1, '\'')),
|
||||
(Some(_), Some('\'')) => Some(i + 3),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
/// The walk itself drops a `#[cfg(test)] mod x;` file, not only the parts
|
||||
/// that find one. A fixture tree, because the wiring is the thing tested.
|
||||
#[test]
|
||||
fn a_test_only_module_file_is_not_production_code() {
|
||||
let root = std::env::temp_dir().join(format!("source_scan_{}", std::process::id()));
|
||||
let src = root.join("src");
|
||||
std::fs::create_dir_all(src.join("t")).unwrap();
|
||||
std::fs::write(src.join("lib.rs"), "mod p;\n#[cfg(test)]\nmod t;\n").unwrap();
|
||||
std::fs::write(src.join("p.rs"), "fn production() {}\n").unwrap();
|
||||
std::fs::write(src.join("t.rs"), "fn test_only() {}\nmod deeper;\n").unwrap();
|
||||
std::fs::write(src.join("t").join("deeper.rs"), "fn test_too() {}\n").unwrap();
|
||||
let mut found: Vec<String> = production_code(std::slice::from_ref(&src))
|
||||
.into_iter()
|
||||
.map(|(file, _)| file.strip_prefix(&src).unwrap().display().to_string())
|
||||
.collect();
|
||||
found.sort();
|
||||
std::fs::remove_dir_all(&root).unwrap();
|
||||
assert_eq!(found, vec!["lib.rs", "p.rs"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_only_items_and_modules_are_not_production() {
|
||||
let text = concat!(
|
||||
"fn kept() {}\n",
|
||||
"#[cfg(test)]\n",
|
||||
"mod inline {\n",
|
||||
" fn gone_inline() {}\n",
|
||||
"}\n",
|
||||
"#[cfg(test)]\n",
|
||||
"mod out_of_line;\n",
|
||||
"#[cfg(any(test, feature = \"x\"))]\n",
|
||||
"pub mod gated;\n",
|
||||
"#[cfg(not(test))]\n",
|
||||
"fn production_only() {}\n",
|
||||
"fn also_kept() {}\n",
|
||||
);
|
||||
let code = code_only(text);
|
||||
let stripped = strip_test_items(&code);
|
||||
for kept in ["fn kept", "fn production_only", "fn also_kept"] {
|
||||
assert!(stripped.contains(kept), "{kept} was dropped: {stripped}");
|
||||
}
|
||||
assert!(!stripped.contains("gone_inline"), "{stripped}");
|
||||
let lib = Path::new("/c/src/lib.rs");
|
||||
assert_eq!(
|
||||
test_module_paths(lib, &code),
|
||||
vec![
|
||||
PathBuf::from("/c/src/out_of_line"),
|
||||
PathBuf::from("/c/src/gated")
|
||||
]
|
||||
);
|
||||
let nested = Path::new("/c/src/ui/reduce.rs");
|
||||
assert_eq!(
|
||||
test_module_paths(nested, &code)[0],
|
||||
PathBuf::from("/c/src/ui/reduce/out_of_line")
|
||||
);
|
||||
assert!(is_module_file(
|
||||
Path::new("/c/src/gated"),
|
||||
Path::new("/c/src/gated.rs")
|
||||
));
|
||||
assert!(is_module_file(
|
||||
Path::new("/c/src/gated"),
|
||||
Path::new("/c/src/gated/a.rs")
|
||||
));
|
||||
assert!(!is_module_file(
|
||||
Path::new("/c/src/gated"),
|
||||
Path::new("/c/src/gatedx.rs")
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn comments_and_literals_are_blanked_and_code_is_kept() {
|
||||
let text = concat!(
|
||||
"let a = Command::Kept(1); // Command::LineComment(2)\n",
|
||||
"/* Command::Block( /* nested */ Command::StillBlock ) */\n",
|
||||
"let s = \"Command::InString(\\\"x\\\")\";\n",
|
||||
"let r = r#\"Command::InRaw(\"q\")\"#;\n",
|
||||
"let q = '\"'; let l: &'static str = \"\"; Command::AfterChar(3)\n",
|
||||
);
|
||||
let kept = code_only(text);
|
||||
assert!(kept.contains("Command::Kept(1)"));
|
||||
assert!(kept.contains("Command::AfterChar(3)"));
|
||||
assert!(
|
||||
kept.contains("&'static str"),
|
||||
"a lifetime is not a char literal"
|
||||
);
|
||||
for gone in ["LineComment", "Block", "StillBlock", "InString", "InRaw"] {
|
||||
assert!(!kept.contains(gone), "{gone} survived: {kept}");
|
||||
}
|
||||
assert_eq!(kept.lines().count(), text.lines().count());
|
||||
}
|
||||
}
|
||||
@@ -5,6 +5,7 @@ use std::collections::BTreeSet;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
use crate::reduce::Command;
|
||||
use crate::source_scan::{production_code, production_text};
|
||||
|
||||
/// A variant production composes nowhere, and why. Checked in **both**
|
||||
/// directions, so a composed entry fails here too (improvise-m0sx).
|
||||
@@ -62,6 +63,24 @@ fn the_uncomposed_list_names_only_real_variants() {
|
||||
);
|
||||
}
|
||||
|
||||
/// The #160 review's fault: a variant named only in a comment or a string
|
||||
/// is not composed. See `context/test-evidence/protocol.md`.
|
||||
#[test]
|
||||
fn a_variant_named_only_in_a_comment_or_string_is_not_composed() {
|
||||
let text = concat!(
|
||||
"fn f() {\n",
|
||||
" // TODO: Command::SetYanked(value)\n",
|
||||
" /* Command::SetSearchMode(true) */\n",
|
||||
" let s = \"Command::ChangeMode(AppMode::Normal)\";\n",
|
||||
"}\n",
|
||||
);
|
||||
assert_eq!(composers_in(text), Vec::<String>::new());
|
||||
assert_eq!(
|
||||
composers_in("let c = Command::SetYanked(None);"),
|
||||
vec!["SetYanked"]
|
||||
);
|
||||
}
|
||||
|
||||
/// The scan must find *something*, or a broken walk reads as a clean bill of
|
||||
/// health — the failure direction `check_the_checker` exists for.
|
||||
#[test]
|
||||
@@ -74,21 +93,22 @@ fn the_scan_reaches_production_code_at_all() {
|
||||
);
|
||||
}
|
||||
|
||||
/// Every `Command::<Name>` in a **value** position, in non-test source.
|
||||
fn composed_in_production() -> BTreeSet<String> {
|
||||
let mut found = BTreeSet::new();
|
||||
for file in production_sources() {
|
||||
let text = std::fs::read_to_string(&file).expect("a listed source reads");
|
||||
for name in constructed_in(&strip_test_items(&text)) {
|
||||
found.insert(name);
|
||||
}
|
||||
}
|
||||
found
|
||||
/// One file's composers: no comment, literal or test item is one (improvise-c7mu).
|
||||
fn composers_in(text: &str) -> Vec<String> {
|
||||
constructed_in(&production_text(text))
|
||||
}
|
||||
|
||||
/// Rust under `src/`, workspace-wide. `tests/` is excluded by construction:
|
||||
/// Every `Command::<Name>` in a **value** position, in production source.
|
||||
fn composed_in_production() -> BTreeSet<String> {
|
||||
production_code(&source_roots())
|
||||
.iter()
|
||||
.flat_map(|(_, code)| constructed_in(code))
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// `src/` of every workspace crate. `tests/` is excluded by construction:
|
||||
/// a variant only a test composes is exactly what this file looks for.
|
||||
fn production_sources() -> Vec<PathBuf> {
|
||||
fn source_roots() -> Vec<PathBuf> {
|
||||
let root = Path::new(env!("CARGO_MANIFEST_DIR"))
|
||||
.parent()
|
||||
.and_then(Path::parent)
|
||||
@@ -98,53 +118,7 @@ fn production_sources() -> Vec<PathBuf> {
|
||||
for entry in std::fs::read_dir(root.join("crates")).expect("crates/ is readable") {
|
||||
roots.push(entry.expect("a crates/ entry reads").path().join("src"));
|
||||
}
|
||||
let mut files = Vec::new();
|
||||
for dir in roots {
|
||||
collect_rust(&dir, &mut files);
|
||||
}
|
||||
files
|
||||
}
|
||||
|
||||
fn collect_rust(dir: &Path, into: &mut Vec<PathBuf>) {
|
||||
let Ok(entries) = std::fs::read_dir(dir) else {
|
||||
return;
|
||||
};
|
||||
for entry in entries.flatten() {
|
||||
let path = entry.path();
|
||||
match path.is_dir() {
|
||||
true => collect_rust(&path, into),
|
||||
false if path.extension().is_some_and(|e| e == "rs") => into.push(path),
|
||||
false => {}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Drop every `#[cfg(test)]` item. rustfmt puts an item's closing brace at the
|
||||
/// attribute's own indentation, which is what makes this a line scan (§2).
|
||||
fn strip_test_items(text: &str) -> String {
|
||||
let lines: Vec<&str> = text.lines().collect();
|
||||
let mut kept = String::new();
|
||||
let mut i = 0;
|
||||
while i < lines.len() {
|
||||
let line = lines[i];
|
||||
match line.trim_start().starts_with("#[cfg(test)]") {
|
||||
false => {
|
||||
kept.push_str(line);
|
||||
kept.push('\n');
|
||||
i += 1;
|
||||
}
|
||||
true => {
|
||||
let indent = line.len() - line.trim_start().len();
|
||||
let close = format!("{}}}", " ".repeat(indent));
|
||||
i += 1;
|
||||
while i < lines.len() && lines[i] != close {
|
||||
i += 1;
|
||||
}
|
||||
i += 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
kept
|
||||
roots
|
||||
}
|
||||
|
||||
/// `Command::<Name>` occurrences that are not `match` patterns. A pattern is
|
||||
|
||||
@@ -42,11 +42,11 @@ pub struct RenderModel {
|
||||
pub categories: Vec<RenderCatRow>,
|
||||
/// Whether the panel is open, so the page does not decide.
|
||||
pub category_panel_open: bool,
|
||||
/// The view panel's rows, **empty unless it is open** — the same rule the
|
||||
/// help overlay follows, so the page asks no question about the mode.
|
||||
pub views: Vec<RenderViewRow>,
|
||||
/// The formula panel's rows, empty unless it is open.
|
||||
pub formulas: Vec<RenderFormulaRow>,
|
||||
/// The view panel's rows, **`None` when it is closed** and possibly empty
|
||||
/// when open: an absence is not an empty list (improvise-df2w).
|
||||
pub views: Option<Vec<RenderViewRow>>,
|
||||
/// The formula panel's rows, on the same terms as `views`.
|
||||
pub formulas: Option<Vec<RenderFormulaRow>>,
|
||||
/// The tile bar, drawn on every frame as the TUI draws it. Which tile is
|
||||
/// `selected` is the mode's answer, not the page's.
|
||||
pub tiles: Vec<RenderTile>,
|
||||
@@ -341,41 +341,39 @@ fn page_axes(state: &ClientState) -> Vec<RenderPageAxis> {
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// The view panel's rows while it is open. The axis line is
|
||||
/// The view panel's rows, or `None` while it is closed. The axis line is
|
||||
/// `viewmodel::view_axis_summary`, which the TUI's own panel calls too.
|
||||
fn view_rows(state: &ClientState) -> Vec<RenderViewRow> {
|
||||
if !state.view.view_panel_open {
|
||||
return Vec::new();
|
||||
}
|
||||
state
|
||||
.views
|
||||
.views
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(i, summary)| RenderViewRow {
|
||||
name: summary.name.clone(),
|
||||
axes: improvise_protocol::viewmodel::view_axis_summary(summary),
|
||||
active: summary.name == state.view.active_view,
|
||||
selected: i == state.view.view_panel_cursor,
|
||||
})
|
||||
.collect()
|
||||
fn view_rows(state: &ClientState) -> Option<Vec<RenderViewRow>> {
|
||||
state.view.view_panel_open.then(|| {
|
||||
state
|
||||
.views
|
||||
.views
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(i, summary)| RenderViewRow {
|
||||
name: summary.name.clone(),
|
||||
axes: improvise_protocol::viewmodel::view_axis_summary(summary),
|
||||
active: summary.name == state.view.active_view,
|
||||
selected: i == state.view.view_panel_cursor,
|
||||
})
|
||||
.collect()
|
||||
})
|
||||
}
|
||||
|
||||
/// The formula panel's rows while it is open.
|
||||
fn formula_rows(state: &ClientState) -> Vec<RenderFormulaRow> {
|
||||
if !state.view.formula_panel_open {
|
||||
return Vec::new();
|
||||
}
|
||||
state
|
||||
.formulas
|
||||
.formulas
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(i, text)| RenderFormulaRow {
|
||||
text: text.clone(),
|
||||
selected: i == state.view.formula_cursor,
|
||||
})
|
||||
.collect()
|
||||
/// The formula panel's rows, or `None` while it is closed.
|
||||
fn formula_rows(state: &ClientState) -> Option<Vec<RenderFormulaRow>> {
|
||||
state.view.formula_panel_open.then(|| {
|
||||
state
|
||||
.formulas
|
||||
.formulas
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(i, text)| RenderFormulaRow {
|
||||
text: text.clone(),
|
||||
selected: i == state.view.formula_cursor,
|
||||
})
|
||||
.collect()
|
||||
})
|
||||
}
|
||||
|
||||
/// The tile bar, always. Only the *selection* is conditional, exactly as
|
||||
@@ -797,34 +795,48 @@ mod tests {
|
||||
client
|
||||
}
|
||||
|
||||
/// An empty list is what says a panel is closed — the same rule the help
|
||||
/// overlay follows, so the page asks no question about the mode.
|
||||
/// `None` is what says a panel is closed, so the page asks no question
|
||||
/// about the mode.
|
||||
#[test]
|
||||
fn a_closed_panel_projects_no_rows_and_an_open_one_does() {
|
||||
let mut client = client_with_panels();
|
||||
let closed = client.model();
|
||||
assert!(
|
||||
closed.views.is_empty(),
|
||||
closed.views.is_none(),
|
||||
"the cache is full but the panel is shut"
|
||||
);
|
||||
assert!(closed.formulas.is_empty());
|
||||
assert!(closed.formulas.is_none());
|
||||
|
||||
client.state_mut().view.view_panel_open = true;
|
||||
client.state_mut().view.formula_panel_open = true;
|
||||
client.state_mut().view.view_panel_cursor = 1;
|
||||
client.state_mut().view.active_view = "Default".into();
|
||||
let open = client.model();
|
||||
assert_eq!(open.views.len(), 2);
|
||||
let (views, formulas) = (open.views.unwrap(), open.formulas.unwrap());
|
||||
assert_eq!(views.len(), 2);
|
||||
assert_eq!(
|
||||
open.views[0].axes, "R:Region",
|
||||
views[0].axes, "R:Region",
|
||||
"the axis line is the shared derivation, and _Dim is not part of it"
|
||||
);
|
||||
assert!(
|
||||
open.views[0].active,
|
||||
"active is ViewState's, not the cache's"
|
||||
);
|
||||
assert!(open.views[1].selected, "and selected is the panel cursor's");
|
||||
assert_eq!(open.formulas[0].text, "Profit = Revenue - Cost");
|
||||
assert!(views[0].active, "active is ViewState's, not the cache's");
|
||||
assert!(views[1].selected, "and selected is the panel cursor's");
|
||||
assert_eq!(formulas[0].text, "Profit = Revenue - Cost");
|
||||
}
|
||||
|
||||
/// Open with nothing in it is not closed: the page must be able to tell
|
||||
/// them apart from the model alone (improvise-df2w).
|
||||
#[test]
|
||||
fn an_open_empty_panel_is_not_a_closed_one() {
|
||||
let mut client = init();
|
||||
let closed = client.model();
|
||||
client.state_mut().view.formula_panel_open = true;
|
||||
let formulas_open = client.model();
|
||||
assert_ne!(formulas_open, closed, "an open, empty formula panel");
|
||||
|
||||
client.state_mut().view.formula_panel_open = false;
|
||||
client.state_mut().view.view_panel_open = true;
|
||||
let views_open = client.model();
|
||||
assert_ne!(views_open, closed, "an open, empty view panel");
|
||||
}
|
||||
|
||||
/// The tile bar is drawn always; only the *selection* waits for the mode,
|
||||
|
||||
@@ -1581,3 +1581,177 @@ fn opens_an_editor(client: &ClientState, key: KeyEvent) -> bool {
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
// Yank and paste (improvise-dsxq).
|
||||
|
||||
fn region_measure(region: &str, measure: &str) -> improvise_command::model::cell::CellKey {
|
||||
improvise_command::model::cell::CellKey::new(vec![
|
||||
("Region".to_string(), region.to_string()),
|
||||
("_Measure".to_string(), measure.to_string()),
|
||||
])
|
||||
}
|
||||
|
||||
/// Four rows and four measures, the last a formula, holding values on which the
|
||||
/// edit text and the value disagree. Evidence file.
|
||||
fn yank_host() -> App {
|
||||
use improvise_protocol::CellValue::{Number, Text};
|
||||
let mut wb = Workbook::new("Test");
|
||||
wb.add_category("Region").unwrap();
|
||||
for item in ["West", "North", "South", "East"] {
|
||||
wb.add_item("Region", item).unwrap();
|
||||
}
|
||||
for item in ["Units", "Revenue", "Cost"] {
|
||||
wb.add_item("_Measure", item).unwrap();
|
||||
}
|
||||
wb.model.add_formula(
|
||||
improvise_command::formula::parse_formula("Profit = Revenue - Cost", "_Measure").unwrap(),
|
||||
);
|
||||
wb.active_view_mut().set_axis("Region", Axis::Row);
|
||||
wb.active_view_mut().set_axis("_Measure", Axis::Column);
|
||||
for (region, measure, value) in [
|
||||
("North", "Revenue", Number(1.0)),
|
||||
("North", "Cost", Number(2.0)),
|
||||
("South", "Revenue", Number(1.23456789)),
|
||||
("South", "Cost", Text("007".into())),
|
||||
("East", "Revenue", Number(10.123456)),
|
||||
("East", "Cost", Number(0.5)),
|
||||
] {
|
||||
wb.model.set_cell(region_measure(region, measure), value);
|
||||
}
|
||||
let mut app = App::new(wb, None);
|
||||
app.term_height = 4 + 8;
|
||||
app.rebuild_layout();
|
||||
app
|
||||
}
|
||||
|
||||
/// One unmodified character on both ends: the host through its keymap, a twin
|
||||
/// through exactly what the client composed.
|
||||
fn press(app: &mut App, twin: &mut App, client: &mut ClientState, key: char) {
|
||||
let event = KeyEvent::new(KeyCode::Char(key), KeyModifiers::NONE);
|
||||
let KeyOutcome::Local(cmds) = resolve(client, event) else {
|
||||
panic!("{key}: the client must answer this key locally");
|
||||
};
|
||||
for cmd in &cmds {
|
||||
client.fold(cmd);
|
||||
}
|
||||
apply_on(twin, &cmds);
|
||||
reduce_key(app, event);
|
||||
}
|
||||
|
||||
fn seat(app: &mut App, client: &mut ClientState, at: (usize, usize)) {
|
||||
app.cursor_mut().selected = at;
|
||||
client.view.cursor_mut().selected = at;
|
||||
}
|
||||
|
||||
/// `yy` then `p` copies the value the TUI copies, not its edit text re-parsed.
|
||||
/// Never an integer: the one kind on which the two agree. Evidence file.
|
||||
#[test]
|
||||
fn yy_then_p_copies_the_value_the_host_copies() {
|
||||
use improvise_protocol::CellValue::{Number, Text};
|
||||
let cases = [
|
||||
(
|
||||
"a non-whole number",
|
||||
(2, 1),
|
||||
(1, 1),
|
||||
"Revenue",
|
||||
Number(1.23456789),
|
||||
),
|
||||
(
|
||||
"number-like text",
|
||||
(2, 2),
|
||||
(1, 2),
|
||||
"Cost",
|
||||
Text("007".into()),
|
||||
),
|
||||
(
|
||||
"a formula's value",
|
||||
(3, 3),
|
||||
(1, 1),
|
||||
"Revenue",
|
||||
Number(10.123456 - 0.5),
|
||||
),
|
||||
];
|
||||
for (what, from, to, measure, expected) in cases {
|
||||
let mut app = yank_host();
|
||||
let mut twin = yank_host();
|
||||
let mut client = client_ready_to_edit(&app, (4, 4));
|
||||
|
||||
seat(&mut app, &mut client, from);
|
||||
press(&mut app, &mut twin, &mut client, 'y');
|
||||
press(&mut app, &mut twin, &mut client, 'y');
|
||||
assert_eq!(
|
||||
app.view_state.yanked,
|
||||
Some(expected.clone()),
|
||||
"{what}: the host must yank the value itself, or the fixture proves nothing"
|
||||
);
|
||||
assert_eq!(
|
||||
client.view.yanked, app.view_state.yanked,
|
||||
"{what}: client and host yanked different values"
|
||||
);
|
||||
|
||||
seat(&mut app, &mut client, to);
|
||||
press(&mut app, &mut twin, &mut client, 'p');
|
||||
assert_eq!(
|
||||
app.model_state
|
||||
.workbook
|
||||
.model
|
||||
.get_cell(®ion_measure("North", measure)),
|
||||
Some(&expected),
|
||||
"{what}: the host's paste must have landed"
|
||||
);
|
||||
assert_eq!(
|
||||
document_text(&twin),
|
||||
document_text(&app),
|
||||
"{what}: the client's paste wrote a different document"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// A true aggregate: `Product` on no axis, so the cell sums two records and has
|
||||
/// no stored value. Yank only: an all-aggregate grid has nowhere to paste.
|
||||
#[test]
|
||||
fn yy_over_an_aggregate_copies_the_sum_the_host_copies() {
|
||||
let mut wb = Workbook::new("Test");
|
||||
wb.add_category("Region").unwrap();
|
||||
wb.add_item("Region", "North").unwrap();
|
||||
wb.add_item("Region", "South").unwrap();
|
||||
wb.add_category("Product").unwrap();
|
||||
wb.add_item("Product", "Widget").unwrap();
|
||||
wb.add_item("Product", "Gadget").unwrap();
|
||||
wb.add_item("_Measure", "Revenue").unwrap();
|
||||
wb.active_view_mut().set_axis("Region", Axis::Row);
|
||||
wb.active_view_mut().set_axis("_Measure", Axis::Column);
|
||||
wb.active_view_mut().set_axis("Product", Axis::None);
|
||||
for (product, value) in [("Widget", 1.23456789), ("Gadget", 1.0)] {
|
||||
wb.model.set_cell(
|
||||
region_measure("South", "Revenue").with("Product", product),
|
||||
improvise_protocol::CellValue::Number(value),
|
||||
);
|
||||
}
|
||||
let mut app = App::new(wb, None);
|
||||
app.term_height = 2 + 8;
|
||||
app.rebuild_layout();
|
||||
let mut client = client_of(&app, (2, 1));
|
||||
|
||||
app.cursor_mut().selected = (1, 0);
|
||||
client.view.cursor_mut().selected = (1, 0);
|
||||
for _ in 0..2 {
|
||||
let event = KeyEvent::new(KeyCode::Char('y'), KeyModifiers::NONE);
|
||||
let KeyOutcome::Local(cmds) = resolve(&client, event) else {
|
||||
panic!("y must resolve locally over an aggregate too");
|
||||
};
|
||||
for cmd in &cmds {
|
||||
client.fold(cmd);
|
||||
}
|
||||
reduce_key(&mut app, event);
|
||||
}
|
||||
assert_eq!(
|
||||
app.view_state.yanked,
|
||||
Some(improvise_protocol::CellValue::Number(1.23456789 + 1.0)),
|
||||
"the host yanks the sum, or the fixture is not an aggregate"
|
||||
);
|
||||
assert_eq!(
|
||||
client.view.yanked, app.view_state.yanked,
|
||||
"client and host yanked different values over an aggregate"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -70,7 +70,7 @@ test.describe("every field the render model projects reaches the screen", () =>
|
||||
await expect(page.locator("#categories")).toBeVisible();
|
||||
await page.keyboard.press("Escape");
|
||||
|
||||
// `views`: empty unless open, so the page asks no question about the mode.
|
||||
// `views`: `null` unless open, so the page asks no question about the mode.
|
||||
await expect(page.locator("#views")).toBeHidden();
|
||||
await page.keyboard.press("Shift+V");
|
||||
await expect(page.locator("#views li")).toHaveCount(2);
|
||||
@@ -356,6 +356,27 @@ test.describe("a pointer event reaches the client and its answer is painted", ()
|
||||
});
|
||||
});
|
||||
|
||||
test.describe("an open panel is drawn even when it has nothing in it", () => {
|
||||
// improvise-df2w: `formulas` said "closed" by being empty, so a document with
|
||||
// no formulas could open the panel and never see it. `tall.improv` has none.
|
||||
test("the formula panel opens over a document with no formulas", async ({ page }) => {
|
||||
await page.goto("/app/index.html?doc=df2w-empty.improv&seed=tall.improv");
|
||||
await expect(page.locator("#grid table")).toBeVisible({ timeout: 20_000 });
|
||||
await page.locator("#grid").focus();
|
||||
await expect(page.locator("#formulas")).toBeHidden();
|
||||
|
||||
await page.keyboard.press("Shift+F");
|
||||
await expect(page.locator("#mode")).toHaveText("FORMULAS");
|
||||
await expect(page.locator("#formulas")).toBeVisible();
|
||||
await expect(page.locator("#formulas li")).toHaveCount(0);
|
||||
|
||||
// And closed is still closed: the other direction, which a page that
|
||||
// simply never hid the panel would fail.
|
||||
await page.keyboard.press("Shift+F");
|
||||
await expect(page.locator("#formulas")).toBeHidden();
|
||||
});
|
||||
});
|
||||
|
||||
test.describe("the selection is on screen", () => {
|
||||
// improvise-jq8l: `web/app.js` called `scrollIntoView` NOWHERE, so a selection
|
||||
// moved past the bottom of `#grid` simply stayed there, invisible, with no way
|
||||
|
||||
@@ -76,36 +76,37 @@ run_case() {
|
||||
scaffold
|
||||
run_case "both workspaces clean is a pass" 0 "$work/repo"
|
||||
|
||||
# The load-bearing case. `cargo fmt --all` at the outer root cannot reach this
|
||||
# file, and `cargo fmt -p inner-pkg` from the outer root exits 1 saying `not a
|
||||
# member of the workspace` -- the usage error this whole script exists to split
|
||||
# from a real verdict.
|
||||
# One fault case PER PACKAGE, enumerated from the fixture rather than
|
||||
# hand-picked: three hand-picked shapes each missed a rival, and the fourth
|
||||
# missed the root package itself (improvise-90ea). Reasoning:
|
||||
# context/test-evidence/tooling.md. The enumeration is a find over
|
||||
# `[package]` manifests, deliberately NOT the script's own `[workspace]` +
|
||||
# `cargo metadata` discovery, which is the thing under test.
|
||||
scaffold
|
||||
printf '%s' "$DIRTY" > "$work/repo/nested/src/lib.rs"
|
||||
if run_case "a fault in the NESTED workspace is reported" 1 "$work/repo"; then
|
||||
case "$OUT" in
|
||||
*nested/src/lib.rs*) ok " and the nested file is named" ;;
|
||||
*) bad " and the nested file is named" "output did not name it: ${OUT:0:300}" ;;
|
||||
esac
|
||||
pkg_manifests=()
|
||||
while IFS= read -r m; do pkg_manifests+=("$m"); done < <(
|
||||
cd "$work/repo" && grep -rl --include=Cargo.toml '^\[package\]' . | sort)
|
||||
# An empty or shrunken enumeration would make the loop below vacuous.
|
||||
if [ "${#pkg_manifests[@]}" = 3 ]; then
|
||||
ok "the fixture enumerates all three packages (root, member, nested)"
|
||||
else
|
||||
bad "the fixture enumerates all three packages" "found: ${pkg_manifests[*]:-none}"
|
||||
fi
|
||||
|
||||
scaffold
|
||||
printf '%s' "$DIRTY" > "$work/repo/a/src/lib.rs"
|
||||
if run_case "a fault in the OUTER workspace is reported" 1 "$work/repo"; then
|
||||
case "$OUT" in
|
||||
*a/src/lib.rs*) ok " and the outer file is named" ;;
|
||||
*) bad " and the outer file is named" "output did not name it: ${OUT:0:300}" ;;
|
||||
esac
|
||||
# The rival: spelling the per-root check `--all` instead of `-p <members>`
|
||||
# walks PAST the nested workspace boundary (measured), so the OUTER file is
|
||||
# reported a second time under the inner root. Counting is what sees that.
|
||||
n="$(printf '%s\n' "$OUT" | grep -c 'a/src/lib.rs')"
|
||||
if [ "$n" = 1 ]; then
|
||||
ok " and it is named ONCE -- no root reaches past its own boundary"
|
||||
else
|
||||
bad " and it is named ONCE" "named $n times; a root is walking past its boundary"
|
||||
for m in "${pkg_manifests[@]}"; do
|
||||
rel="$(dirname "${m#./}")/src/lib.rs"; rel="${rel#./}"
|
||||
scaffold
|
||||
printf '%s' "$DIRTY" > "$work/repo/$rel"
|
||||
if run_case "a fault in $rel is reported" 1 "$work/repo"; then
|
||||
# Counting, not matching: `--all` for `-p` walks PAST the nested boundary
|
||||
# (measured) and names an outer file a second time under the inner root.
|
||||
n="$(printf '%s\n' "$OUT" | grep -cF "$work/repo/$rel")"
|
||||
if [ "$n" = 1 ]; then
|
||||
ok " and $rel is named exactly once"
|
||||
else
|
||||
bad " and $rel is named exactly once" "named $n times; output: ${OUT:0:300}"
|
||||
fi
|
||||
fi
|
||||
fi
|
||||
done
|
||||
|
||||
# A usage error must NOT wear a formatting failure's exit code. This is the
|
||||
# whole ticket: `cargo fmt -p opfs-contract` from the repo root exits 1.
|
||||
@@ -121,6 +122,32 @@ run_case "a tree with no workspace root is a usage error, not a pass" 2 "$work/e
|
||||
|
||||
run_case "--help is a usage error, so a typo cannot pass" 2 "$work/repo" --help
|
||||
|
||||
# The loop above covers every package the FIXTURE has; this covers every
|
||||
# package the TREE has. A package no root lists is checked by nothing, so the
|
||||
# gate must refuse a verdict -- and name the package -- rather than pass.
|
||||
scaffold
|
||||
mkdir -p "$work/repo/orphan/src"
|
||||
printf '[package]\nname = "orphan-pkg"\nversion = "0.1.0"\nedition = "2021"\n' \
|
||||
> "$work/repo/orphan/Cargo.toml"
|
||||
printf '%s' "$DIRTY" > "$work/repo/orphan/src/lib.rs"
|
||||
cat > "$work/repo/Cargo.toml" <<'EOF'
|
||||
[workspace]
|
||||
members = ["a"]
|
||||
exclude = ["orphan"]
|
||||
resolver = "2"
|
||||
|
||||
[package]
|
||||
name = "root-pkg"
|
||||
version = "0.1.0"
|
||||
edition = "2021"
|
||||
EOF
|
||||
if run_case "a package no workspace root reaches is exit 2, not a pass" 2 "$work/repo"; then
|
||||
case "$OUT" in
|
||||
*"no workspace root reaches"*orphan/Cargo.toml*) ok " and the unreached package is named" ;;
|
||||
*) bad " and the unreached package is named" "output: ${OUT:0:300}" ;;
|
||||
esac
|
||||
fi
|
||||
|
||||
# The narrowed contract, pinned so it is not merely prose (ChatGPT Codex on
|
||||
# PR #159). rustfmt exits 1 for a source it cannot parse, exactly as for a
|
||||
# diff -- so exit 1 is "rustfmt did not certify this root clean", and what
|
||||
|
||||
+43
-17
@@ -1,7 +1,7 @@
|
||||
#!/usr/bin/env bash
|
||||
# Format-checks every Cargo workspace root in the repo, from any directory.
|
||||
# `cargo fmt --all` reaches only ONE of them. Reasoning: context/code/tooling.md
|
||||
# section 4 -- including why `-p` scoped by `--manifest-path` is the only
|
||||
# `cargo fmt --all` reaches only ONE of them. Reasoning: context/test-evidence/
|
||||
# tooling.md, "The two-workspace fmt gate" -- including why `-p` scoped by `--manifest-path` is the only
|
||||
# spelling that is neither blind nor boundary-crossing (improvise-b4ks).
|
||||
set -uo pipefail
|
||||
|
||||
@@ -18,8 +18,9 @@ exit 1 rustfmt did not certify a root clean: a file differs from rustfmt's
|
||||
output, or rustfmt refused to parse one. Either way a FILE IS NAMED
|
||||
and the tree is what is wrong.
|
||||
exit 2 the gate did not run: usage error, no cargo, no rustfmt, cargo
|
||||
metadata failed, or no [workspace] root found. NO verdict was
|
||||
produced and NO file is named.
|
||||
metadata failed, no [workspace] root found, or a [package] that no
|
||||
root has as a member. NO verdict was produced. No SOURCE file is
|
||||
named; the unreached-package case names its Cargo.toml.
|
||||
|
||||
The split is "did the gate run", not "is every exit-1 cause a diff". That is
|
||||
narrower than it sounds and it is deliberate -- see the note on rc=1 below.
|
||||
@@ -38,8 +39,8 @@ EOF
|
||||
# The find is RELATIVE to the tree being scanned: an absolute `-path` filter
|
||||
# matches the ancestors too, and this repo is itself checked out under
|
||||
# `.claude/worktrees/` in every agent worktree, where that excluded everything.
|
||||
workspace_roots() {
|
||||
local root="$1"
|
||||
manifests_with() {
|
||||
local root="$1" section="$2"
|
||||
( cd "$root" && find . -name Cargo.toml \
|
||||
-not -path './target/*' -not -path '*/target/*' \
|
||||
-not -path './.git/*' \
|
||||
@@ -47,11 +48,12 @@ workspace_roots() {
|
||||
-not -path './.claude/*' ) \
|
||||
| sort \
|
||||
| while IFS= read -r rel; do
|
||||
if grep -q '^\[workspace\]' "$root/${rel#./}"; then
|
||||
if grep -q "^\\[$section\\]" "$root/${rel#./}"; then
|
||||
printf '%s\n' "$root/${rel#./}"
|
||||
fi
|
||||
done
|
||||
}
|
||||
workspace_roots() { manifests_with "$1" workspace; }
|
||||
|
||||
# The members of ONE workspace, named by cargo rather than parsed out of the
|
||||
# manifest: a `members = [...]` list may carry globs and path dependencies.
|
||||
@@ -61,10 +63,10 @@ members_of() {
|
||||
printf '%s' "$meta" | python3 -c '
|
||||
import json, sys
|
||||
doc = json.load(sys.stdin)
|
||||
names = [p["name"] for p in doc["packages"]]
|
||||
if not names:
|
||||
rows = [p["name"] + "\t" + p["manifest_path"] for p in doc["packages"]]
|
||||
if not rows:
|
||||
sys.exit(1)
|
||||
print("\n".join(names))
|
||||
print("\n".join(rows))
|
||||
' || return 1
|
||||
}
|
||||
|
||||
@@ -106,15 +108,39 @@ main() {
|
||||
return 2
|
||||
fi
|
||||
|
||||
local rc=0 manifest member
|
||||
for manifest in "${roots[@]}"; do
|
||||
local -a pkgs=()
|
||||
while IFS= read -r member; do [ -n "$member" ] && pkgs+=("$member"); done \
|
||||
< <(members_of "$manifest")
|
||||
if [ ${#pkgs[@]} -eq 0 ]; then
|
||||
printf 'fmt-workspaces: cargo metadata failed for %s\n' "$manifest" >&2
|
||||
# Pass 1: every root's members, before anything is formatted, so that a
|
||||
# package NO root reaches is exit 2 rather than silently never checked.
|
||||
local rc=0 manifest member i m
|
||||
local -a members=() covered=()
|
||||
for i in "${!roots[@]}"; do
|
||||
local rows
|
||||
rows="$(members_of "${roots[$i]}")" || rows=""
|
||||
if [ -z "$rows" ]; then
|
||||
printf 'fmt-workspaces: cargo metadata failed for %s\n' "${roots[$i]}" >&2
|
||||
return 2
|
||||
fi
|
||||
members[$i]="$(printf '%s\n' "$rows" | cut -f1 | tr '\n' ' ')"
|
||||
while IFS= read -r m; do covered+=("$m"); done < <(printf '%s\n' "$rows" | cut -f2)
|
||||
done
|
||||
local orphans
|
||||
orphans="$(manifests_with "$root" package | python3 -c '
|
||||
import os, sys
|
||||
covered = {os.path.realpath(p) for p in sys.argv[1:]}
|
||||
for line in sys.stdin:
|
||||
m = line.rstrip("\n")
|
||||
if m and os.path.realpath(m) not in covered:
|
||||
print(m)
|
||||
' "${covered[@]}")"
|
||||
if [ -n "$orphans" ]; then
|
||||
printf 'fmt-workspaces: no workspace root reaches these packages, so nothing checks them:\n%s\n' \
|
||||
"$orphans" >&2
|
||||
return 2
|
||||
fi
|
||||
|
||||
for i in "${!roots[@]}"; do
|
||||
manifest="${roots[$i]}"
|
||||
local -a pkgs=()
|
||||
read -r -a pkgs <<< "${members[$i]}"
|
||||
|
||||
if [ "$list" = 1 ]; then
|
||||
printf '%s: %s\n' "$manifest" "${pkgs[*]}"
|
||||
|
||||
+4
-4
@@ -594,12 +594,12 @@ function edit(td, row, col, seed) {
|
||||
});
|
||||
}
|
||||
|
||||
/** A side panel. An empty row list is what says it is closed — the page asks
|
||||
* no question about the mode, exactly as `paintHelp` does not. */
|
||||
/** A side panel. `null` is what says it is closed, and `[]` is open and empty:
|
||||
* two facts, never inferred from a count (improvise-df2w). */
|
||||
function paintPanel(host, rows, build) {
|
||||
host.hidden = rows.length === 0;
|
||||
host.hidden = rows === null;
|
||||
const list = document.createElement("ul");
|
||||
rows.forEach((row, index) => {
|
||||
(rows ?? []).forEach((row, index) => {
|
||||
const li = build(row);
|
||||
li.dataset.index = String(index);
|
||||
list.appendChild(li);
|
||||
|
||||
Reference in New Issue
Block a user