Merge pull request #192 from levineuwirth/bite-positive-control

tooling: give scripts/bite a positive control, and correct the handoff

docs/active-work.md

@@ -521,6 +521,61 @@ has **no branch and no framing yet**.
   `FrontendView.fold_projection` to `true` for semantic frontends, which
   Stage 2 deliberately left `false` (Q#FD21).
 
+## Test-improvement arc, lane 6 — `scripts/bite` positive control
+
+- Portable branch: `githubsucks/bite-positive-control`, worktree
+  `../pmacs-bite`, based on `main` @ `300cbc4`. Tooling and
+  documentation only — **no product code, no tests changed.**
+- **Self-contained deliberately.** An earlier draft of this entry
+  scoped the lane by citing a `TEST_IMPROVEMENT.md` at the repo root.
+  That file is **untracked and machine-local** — not on `main`, not in
+  this PR's tree — so the citation was a dangling reference for any
+  other machine, which is exactly what this ledger exists to prevent.
+  The lane is described here on its own terms instead. If that audit
+  is to scope later lanes it needs committing first, in its own PR.
+- Sequenced ahead of the rest of the testing work because every later
+  lane's evidence is bite-shaped. A verifier that cannot fail is not
+  worth more than the claims it certifies.
+- **The defect:** the script ran only the swapped tree, so a failing
+  swapped run was the only thing it checked. A test that fails
+  everywhere — a typo, an unrelated compile break — therefore printed
+  `bite: OK`. Now it asserts the named tests pass against the working
+  tree **and** that at least one ran (a filter matching nothing exits
+  0, so passing alone is insufficient), exiting 3 as `NO CONTROL`
+  otherwise, and it labels `OK (assertion)` versus `OK (COMPILE)`.
+- **Validated by execution, not by reading**, and the results are
+  listed with the one arm that was NOT reproduced:
+  - zero-match filter -> `NO CONTROL`, exit 3;
+  - genuine bite -> `control OK` then `OK (assertion)`, exit 0;
+  - test broken so it fails on BOTH trees -> `NO CONTROL`, exit 3.
+    **The old script printed `bite: OK` here**, which is the whole
+    point of the lane;
+  - assertion failure whose own output prints `error[E0308]` and
+    `error: could not compile` at column 0 -> still `OK (assertion)`,
+    which is why classification reads libtest's summary rather than
+    compiler text;
+  - swapped file that will not build -> `OK (COMPILE)`;
+  - **`INCONCLUSIVE (MIXED)` was not reproduced.** Its assumed trigger
+    — `--test A --test B` where B's swapped file fails to build — was
+    tested and **does not reach it**: cargo builds every named target
+    before running any, so B's failure stops A from running and the
+    run yields no summary at all (the COMPILE arm). The arm stays as
+    defence for the genuinely reachable causes (doc-tests failing to
+    compile after lib tests pass; a harness dying after its summary),
+    and the script says it was not manufactured.
+- **Handoff correction, verified rather than inherited.**
+  `docs/agent-handoff.md` §5 claimed the script "restores by `git
+  checkout --`, which reverts the file to HEAD", destroying
+  uncommitted work. False: it copies to a `mktemp` path before the
+  swap and restores from that copy under an `EXIT INT TERM` trap. The
+  commit-before-gating rule stands on other grounds and is kept; the
+  false mechanism is removed, because it would push the next reader
+  toward `git stash` — the repo-global trap the script exists to
+  avoid.
+- Recovery from a clean checkout:
+  `git fetch githubsucks && git worktree add ../pmacs-bite
+  -b bite-positive-control githubsucks/bite-positive-control`.
+
 ## Parked lane: kill-ring browser + persistence
 
 - Portable branch: `githubsucks/kill-ring-browser`

docs/agent-handoff.md

@@ -1569,16 +1569,43 @@ round-trip cannot detect a discriminant shift.
   your tree (happened during #111: a failed `stash push` chained
   into `stash pop`, which grabbed the user's PR-#17-era entry). For
   run-tests-against-an-old-version swaps, use `scripts/bite` — a
-  trap-guarded one-file swap over read-only `git show`, with an
-  inverted verdict (exit 0 iff the tests FAIL against the old
+  trap-guarded one-file swap over read-only `git show`, with a
+  two-sided verdict (the tests must PASS now and FAIL against the old
   version), making bite-verification machine-checkable.
-- **A fix must be COMMITTED before it is bitten.** `scripts/bite`
-  restores by `git checkout --`, which reverts the file to **HEAD**, not
-  to the state it found — so any uncommitted work in a bitten file is
-  destroyed. A whole review round's fixes were wiped this way during
-  #165. Corollary for a NEW file: the swap-over-`git show` mode does not
-  apply at all, so its claims must be bitten by hand-editing, which makes
-  the commit-first rule load-bearing rather than hygienic.
+- **`scripts/bite` is only as good as its positive control, and it
+  had none until it was given one.** Before that it ran *only* the
+  swapped tree, so it could not tell "my fix is load-bearing" from
+  "my test is broken": a test that fails everywhere made the swapped
+  run fail, and a failing swapped run was the only thing checked, so
+  it printed `bite: OK` and certified nothing. It now asserts the
+  named tests pass **and** that at least one actually ran (a filter
+  matching nothing exits 0, so passing alone is not enough), exiting
+  3 as `NO CONTROL` otherwise. It also distinguishes `OK (assertion)`
+  from `OK (COMPILE)` — an old file that does not build against the
+  current tree is a much weaker result, because the tests may never
+  have run at all. **Read which one it printed.**
+- **A fix must be COMMITTED before it is bitten** — but *not* for the
+  reason previously recorded here. This file used to say `scripts/bite`
+  "restores by `git checkout --`, which reverts the file to HEAD", and
+  that a review round's fixes were wiped that way during #165. **That
+  mechanism description is false and was verified false:** the script
+  copies the file to a `mktemp` path before the swap and restores from
+  that copy, under an `EXIT INT TERM` trap, so uncommitted work in the
+  bitten file survives. It has never touched git state beyond a
+  read-only `git show`. The rule still stands on its own merits —
+  gate results must describe the pushed tree, and a `cargo fmt` after
+  a commit splits worktree from branch — but do not repeat the
+  destroys-your-work claim, which will push the next reader toward
+  `git stash` to protect themselves, straight into the repo-global
+  trap above. **The #165 incident itself is now unexplained**, and
+  that is recorded rather than papered over: work really was lost, but
+  not by the mechanism this file blamed. A `SIGKILL` bypasses the trap
+  and would leave the swapped file in place, which is one candidate;
+  so is a stash collision, given the same round. Do not invent a
+  mechanism to close the gap — an unexplained incident is safer than a
+  confident wrong cause, which is what produced this correction.
+  Corollary for a NEW file: the swap-over-`git show` mode
+  does not apply at all, so its claims must be bitten by hand-editing.
 - **A CONFLICTING PR silently runs no CI at all.** GitHub builds
   `pull_request` workflow runs against the PR's **merge ref**, which it
   does not create while the branch conflicts with its base. So pushes

scripts/bite

@@ -1,6 +1,7 @@
 #!/bin/sh
-# scripts/bite --- prove a fix's tests BITE: run them against an older
-# version of ONE file and succeed only if they FAIL there.
+# scripts/bite --- prove a fix's tests BITE: run them against the
+# working tree (they must PASS), then against an older version of ONE
+# file (they must FAIL).
 #
 #   scripts/bite <ref> <path> [cargo-test-args...]
 #
@@ -8,21 +9,52 @@
 #   scripts/bite HEAD~1 builtin/runtime/editops.lua \
 #       --test editops_acceptance -- capitalize trim_on_save_unexpected
 #
-# Exit status: 0 when the named tests fail against <ref>'s version of
-# <path> (the fix bites), 1 when they still pass (vacuous), 2 on
-# usage/setup errors. The working-tree file is restored on every exit
-# path, including interrupts.
+# Exit status: 0 when the named tests pass now and fail against
+# <ref>'s version of <path> (the fix bites), 1 when they still pass
+# there (vacuous), 2 on usage/setup errors, 3 when the POSITIVE
+# CONTROL fails, 4 when the swapped run is INCONCLUSIVE (see the MIXED
+# arm below --- non-zero because its likeliest cause is vacuity, not
+# success). The working-tree file is restored on every exit path,
+# including interrupts.
 #
-# Caveat: a COMPILE error of the old tree also counts as "fails" —
-# correct, but weaker evidence than a clean assertion failure; eyeball
-# the output when the swapped file is Rust rather than Lua.
+# THE POSITIVE CONTROL, and why it is not optional. Before this
+# existed the script ran only the swapped tree, so it could not
+# distinguish "my fix is load-bearing" from "my test is broken": a
+# typo, an unrelated compile break, or a filter that matches nothing
+# all made the swapped run fail, and a failing swapped run was the
+# only thing it checked. It printed `bite: OK` and certified nothing.
+# So the control asserts two things, because passing alone is not
+# enough — `cargo test` with a filter matching zero tests exits 0, and
+# then a compile error in the old tree would still read as OK:
+#
+#   * the named tests PASS against the working tree, and
+#   * at least one test actually RAN.
+#
+# COMPILE errors of the old tree still count as "fails", and that is
+# correct but weaker than a clean assertion failure --- the swapped
+# file may simply not build against the current tree. The script now
+# says which kind it saw instead of leaving it to be eyeballed. Prefer
+# an assertion failure; treat a compile failure as a prompt to narrow
+# the swap to a file that compiles both ways.
+#
+# That classification reads libtest's own `test result:` summary and
+# deliberately does NOT grep for compiler text. A test may PRINT
+# compiler output on failure --- `compile_mode_acceptance` has a
+# fixture that emits `error[E0308]: mismatched types` at column 0, and
+# the compile-mode and editops suites are this script's primary
+# consumers --- so a `^error[E...]` grep would stamp "weaker evidence"
+# on exactly the strong result it was meant to identify. `test result:`
+# is emitted whenever the harness ran, is absent when the swapped file
+# did not build, and stays uncolored when piped.
 #
 # Why this exists: bite-verification is step 4 of the working method,
-# and the obvious shortcut — git stash — is a trap here. The stash
+# and the obvious shortcut --- git stash --- is a trap here. The stash
 # namespace is REPO-GLOBAL: shared across every worktree and with
 # humans, so a scripted push/pop can collide with (or pop!) someone
 # else's stashed work. This helper never touches git state beyond a
-# read-only `git show`.
+# read-only `git show`, and it restores the working file from a
+# mktemp copy --- NOT from git, so uncommitted work in <path> is
+# preserved.
 
 set -eu
 
@@ -40,6 +72,48 @@ if [ ! -f "$path" ]; then
     exit 2
 fi
 
+# Both cargo runs are CAPTURED rather than streamed, because their
+# output is parsed (passed-counts for the control, `test result:` for
+# the classification). A long acceptance suite therefore prints
+# nothing until it finishes --- that is the trade, not a bug. Do not
+# "fix" it back to streaming without giving the parser another source.
+#
+# Colour is pinned off on both runs, so an ambient CARGO_TERM_COLOR
+# cannot wrap the summary lines in escape codes and silently stop every
+# anchored match here from matching. Note the limit: an explicit
+# `--color always` in the passed-through args still beats the
+# environment. That is self-inflicted, and the failure is fail-closed
+# (a zeroed count reads as NO CONTROL), but do not pass it.
+export CARGO_TERM_COLOR=never
+
+# Sum the `N passed` figures across every `test result:` line, so a
+# multi-target invocation is counted correctly rather than only its
+# last binary.
+count_passed() {
+    sed -n 's/^test result: ok\. \([0-9][0-9]*\) passed.*/\1/p' \
+        | awk '{ total += $1 } END { print total + 0 }'
+}
+
+# --- Positive control: the tests must pass, and must exist, NOW. ---
+echo "bite: positive control --- running against the working tree" >&2
+control_status=0
+control_out=$(cargo test "$@" 2>&1) || control_status=$?
+printf '%s\n' "$control_out"
+
+if [ "$control_status" -ne 0 ]; then
+    echo "bite: NO CONTROL --- tests do not pass against the working tree." >&2
+    echo "bite: fix the tests first; a failure here makes the swapped run meaningless." >&2
+    exit 3
+fi
+
+control_ran=$(printf '%s\n' "$control_out" | count_passed)
+if [ "$control_ran" -eq 0 ]; then
+    echo "bite: NO CONTROL --- the filter matched no tests (0 passed)." >&2
+    echo "bite: check the test names; an empty filter passes everywhere and proves nothing." >&2
+    exit 3
+fi
+echo "bite: control OK --- $control_ran test(s) pass against the working tree" >&2
+
 saved=$(mktemp "${TMPDIR:-/tmp}/bite.XXXXXX")
 cp -- "$path" "$saved"
 restore() {
@@ -52,9 +126,52 @@ trap restore EXIT INT TERM
 # script works from any directory inside the repo.
 git show "$ref:./$path" > "$path"
 
-if cargo test "$@"; then
+echo "bite: swapping in $ref:$path" >&2
+swapped_status=0
+swapped_out=$(cargo test "$@" 2>&1) || swapped_status=$?
+printf '%s\n' "$swapped_out"
+
+if [ "$swapped_status" -eq 0 ]; then
     echo "bite: VACUOUS --- tests still pass against $ref:$path" >&2
     exit 1
 fi
 
-echo "bite: OK --- tests fail against $ref:$path (the fix bites)"
+# Distinguish the ways the swapped run can fail. An assertion failure
+# is the evidence we want; a build failure only says the old file does
+# not compile here, and the tests may never have run at all.
+#
+# Classified from libtest's summary line, NOT from compiler text --- a
+# failing test can print `error[E0308]: ...` at column 0 itself (see
+# the header). `test result: FAILED` means the harness ran and tests
+# failed; no `test result:` line at all means nothing ran.
+if printf '%s\n' "$swapped_out" | grep -q '^test result: FAILED'; then
+    echo "bite: OK (assertion) --- tests fail against $ref:$path (the fix bites)"
+elif printf '%s\n' "$swapped_out" | grep -q '^test result:'; then
+    # Some target ran to completion with no failure, yet cargo still
+    # exited non-zero.
+    #
+    # NOT reachable the obvious way, which was checked rather than
+    # assumed: `--test A --test B` where B's swapped file will not
+    # build does NOT land here. Cargo builds every named target before
+    # running any, so B's build failure stops A from running too and
+    # there is no summary at all --- that is the COMPILE arm below.
+    # What does reach here is a run that prints summaries and *then*
+    # fails: doc-tests failing to compile after the lib tests pass, or
+    # a harness dying after its summary line. This arm is therefore
+    # defensive, and its trigger was not manufactured for validation.
+    #
+    # It exits NON-ZERO on purpose. The likeliest reading is not
+    # success but VACUITY: the named tests ran clean against the old
+    # tree (exactly the vacuous outcome) while an unrelated failure
+    # supplied the exit status. Reporting "OK" and exiting 0 here would
+    # let a scripted caller --- or a framing doc quoting "bite exited
+    # 0" --- record a certified bite for a run that certified nothing.
+    echo "bite: INCONCLUSIVE (MIXED) --- some tests ran, none failed, yet the run failed." >&2
+    echo "bite: the named tests may have PASSED against $ref:$path with an unrelated" >&2
+    echo "bite: build error supplying the exit status. Narrow to one target and re-run." >&2
+    exit 4
+else
+    echo "bite: OK (COMPILE) --- $ref:$path produced no test summary" >&2
+    echo "bite: (build failure, or a harness that died before printing one)." >&2
+    echo "bite: weaker evidence than an assertion failure --- the tests may never have run." >&2
+fi