From 85cf9a3ecab7d44b2d485dc8c6a452a933e1f449 Mon Sep 17 00:00:00 2001 From: Noah Gift Date: Mon, 21 Sep 2026 08:06:06 +0200 Subject: [PATCH] =?UTF-8?q?fix(guard):=20check=5Fpr=5Freview=5Fcounts.sh?= =?UTF-8?q?=20was=20RED=20on=20main=20and=20ran=20nowhere=20=E2=80=94=20a?= =?UTF-8?q?=20comment=20classified=20it=20out=20of=20guard-tree=20(PMAT-36?= =?UTF-8?q?46)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PRQ-013 (#2985) deleted ci.yml's receipt job and, with it, the step names that stated six of the counts this guard checks. The guard went RED ("6 row(s) disagree with the tree") and nobody saw it, because a comment at :196 naming the build tool matched guard_tree.sh's CARGO_RE. That dropped the guard from guard-tree's --no-cargo run, and guard-cargo never names it. - The site table drops the five ci.yml rows whose sentences left with the job (the roadmap records them as a STATED LOSS). ci.yml's "43-row" goes from 2 sites to 1. The bats_tests derivation is removed: no file states that count any more, and a derivation with no site checks nothing. check_pr_review_wiring.sh's "43-row" joins the table as a new site, with a self-test row (stale-wiring-rows). - The :196 comment no longer names the tool, so `guard_tree.sh --dry-run --no-cargo` prints `run: scripts/check_pr_review_counts.sh`. Restoring the old wording moves it back to --cargo-only (mutation-verified). - In --list mode, mutate-guard.sh and mutate_quorum_arm.sh now require only awk. They demanded bats/jq/minisign/check-jsonschema before printing a catalogue that uses none of them, so the counts derivation exited ENV=2 on a runner without them. Reproduced under a PATH without those four: rc=2 before, rc=0 after. guard-tree runs on the clean-room hosts, including the gx10/yoga docker runners, so wiring the guard without this would have swapped a dark guard for a red job. The real sweeps still refuse to run without the tools (rc=1). Verified: check rc=0 (22 rows); --self-test 16/16; guard_tree_test.sh 23/23; check_pr_review_wiring.sh PASS, self-test 11/11; check_guards_are_wired.sh PASS, self-test 7/7. Item 2 of #3646 needs no code. check_pr_review_receipt.sh is invoked with a receipt by check_pr_review_arm4.sh (A3 and A4), which pr-review-quorum.yml runs. The ledger line #3647 adds for this guard is deleted here once #3647 lands (the ledger is shrink-only). Refs #3646 Co-Authored-By: Claude Opus 5 (1M context) --- docs/roadmaps/entries/PMAT-3646.yaml | 23 +++++++++++ docs/roadmaps/roadmap.yaml | 23 +++++++++++ scripts/check_pr_review_counts.sh | 57 +++++++++++++++------------- scripts/mutate-guard.sh | 7 +++- scripts/mutate_quorum_arm.sh | 5 ++- 5 files changed, 86 insertions(+), 29 deletions(-) create mode 100644 docs/roadmaps/entries/PMAT-3646.yaml diff --git a/docs/roadmaps/entries/PMAT-3646.yaml b/docs/roadmaps/entries/PMAT-3646.yaml new file mode 100644 index 0000000000..b978739a78 --- /dev/null +++ b/docs/roadmaps/entries/PMAT-3646.yaml @@ -0,0 +1,23 @@ +- id: PMAT-3646 + github_issue: 3646 + item_type: task + title: check_pr_review_counts.sh is RED on main (6 rows disagree) and check_pr_review_receipt.sh has no caller — both dark since PRQ-013 moved the receipt job, classified out of --no-cargo by a comment + status: planned + priority: high + assigned_to: null + created: 2026-09-21T06:00:51Z + updated: 2026-09-21T06:00:51Z + spec: null + acceptance_criteria: + - 'Issue #3646 "What lands" item 1, verbatim: "Fix the 6 disagreeing rows in PR-REVIEW-SKILL-002''s files (or the derivation), then reword the one comment so the guard leaves the cargo population and `guard-tree` runs it; delete its ledger line in the same commit (shrink-only)."' + - 'Issue #3646 "What lands" item 2, verbatim: "Decide `check_pr_review_receipt.sh`''s caller: either `pr-review-quorum.yml` invokes it on the PR''s receipt (a workflow edit — operator''s call), or it is folded into `check_pr_review_arm4.sh`, which is already wired with args. Delete its ledger line with whichever lands."' + phases: [] + subtasks: [] + estimated_effort: null + labels: + - kind:code + notes: 'Handed over by aprender-b3 (2026-09-21); claimed on the issue. Measured on origin/main @ 237fbc32f per the issue: `bash scripts/check_pr_review_counts.sh` rc=1 "--- 6 row(s) disagree with the tree ---"; `bash scripts/check_pr_review_receipt.sh` rc=1 "A run over zero receipts is not a pass (S6.3: a missing receipt is RED)." The ledger line for each guard is added by #3647 (scripts/unwired_guards_baseline.txt, shrink-only); it is deleted in the commit that wires that guard, after #3647 merges. Item 1 lands first; item 2 lands here only if it folds into check_pr_review_arm4.sh with no workflow edit, otherwise the ticket stays open for the operator''s call and the PR says Refs, not Closes. + + What the item-1 PR does. (a) The 6 RED rows were ci.yml sites whose sentences PRQ-013 deleted along with the receipt job (roadmap: "STATED LOSS"), so the rows go. The bats_tests derivation goes too, because no file states that count any more; ci.yml''s fixture_rows site goes from 2 to 1; and scripts/check_pr_review_wiring.sh''s "43-row" joins the table with a self-test row of its own. (b) The :196 comment is reworded so CARGO_RE stops matching: `guard_tree.sh --dry-run --no-cargo` prints `run: scripts/check_pr_review_counts.sh`, and restoring the old wording moves it back to --cargo-only (mutation-verified). (c) The --list mode of mutate-guard.sh and mutate_quorum_arm.sh requires only awk. Before this it demanded bats/jq/minisign/check-jsonschema, so wiring the counts guard into guard-tree, which runs on clean-room HOSTS including the gx10/yoga docker runners, would have turned a dark guard into an ENV-red job (reproduced: rc=2 under a PATH without those four). Sweeps still refuse without them (rc=1). + + Item 2, measured: check_pr_review_receipt.sh HAS a caller with a receipt. check_pr_review_arm4.sh runs `bash "$guard" "$scratch"` (A3) and `bash "$guard" "$best_dir"` (A4) with guard defaulting to it, and pr-review-quorum.yml:141 runs arm4. So there is nothing to fold. What remains is whether guard_tree.sh and the unwired ledger should treat a guard reachable only through another wired guard as wired. That is recorded on the issue and not changed here.' diff --git a/docs/roadmaps/roadmap.yaml b/docs/roadmaps/roadmap.yaml index 618b9a93ca..68d106c69b 100644 --- a/docs/roadmaps/roadmap.yaml +++ b/docs/roadmaps/roadmap.yaml @@ -18990,3 +18990,26 @@ roadmap: labels: - kind:code notes: 'ACCEPTANCE (hand-entered from issue #3627 done_when; minted on the PR branch so the AD-04 quorum can run -- pmat work status refuses a missing item). Done when: contracts/rmsnorm-kernel-v1.yaml Scale invariance obligation carries either the ε>0 formal or the α²·mean(x²) ≫ ε precondition; pv validate passes ~~regenerated with pv probar --binding~~ **WITHDRAWN 21:40Z** — b3 measured that pv probar --binding emits 7 #[ignore = "no binding available"] stubs for this contract; the tracked file''s 5 real tests were produced by the pre-monorepo external generator (20dda945b, 2026-02-18) and the header is false (class row: #3630). Instead: the tracked test is edited **directly**, the commit says so, and the file''s header is replaced by a true one (hand-maintained, contract-bound) the minimal input above ([0.0, -0.15964308], α=0.012254778) is a committed regression case that PASSES under the new assertion (i.e. the fixed test is exercised on the exact input that killed #3621), and a mutation that restores the ε=0 assertion turns it RED cargo' +- id: PMAT-3646 + github_issue: 3646 + item_type: task + title: check_pr_review_counts.sh is RED on main (6 rows disagree) and check_pr_review_receipt.sh has no caller — both dark since PRQ-013 moved the receipt job, classified out of --no-cargo by a comment + status: planned + priority: high + assigned_to: null + created: 2026-09-21T06:00:51Z + updated: 2026-09-21T06:00:51Z + spec: null + acceptance_criteria: + - 'Issue #3646 "What lands" item 1, verbatim: "Fix the 6 disagreeing rows in PR-REVIEW-SKILL-002''s files (or the derivation), then reword the one comment so the guard leaves the cargo population and `guard-tree` runs it; delete its ledger line in the same commit (shrink-only)."' + - 'Issue #3646 "What lands" item 2, verbatim: "Decide `check_pr_review_receipt.sh`''s caller: either `pr-review-quorum.yml` invokes it on the PR''s receipt (a workflow edit — operator''s call), or it is folded into `check_pr_review_arm4.sh`, which is already wired with args. Delete its ledger line with whichever lands."' + phases: [] + subtasks: [] + estimated_effort: null + labels: + - kind:code + notes: 'Handed over by aprender-b3 (2026-09-21); claimed on the issue. Measured on origin/main @ 237fbc32f per the issue: `bash scripts/check_pr_review_counts.sh` rc=1 "--- 6 row(s) disagree with the tree ---"; `bash scripts/check_pr_review_receipt.sh` rc=1 "A run over zero receipts is not a pass (S6.3: a missing receipt is RED)." The ledger line for each guard is added by #3647 (scripts/unwired_guards_baseline.txt, shrink-only); it is deleted in the commit that wires that guard, after #3647 merges. Item 1 lands first; item 2 lands here only if it folds into check_pr_review_arm4.sh with no workflow edit, otherwise the ticket stays open for the operator''s call and the PR says Refs, not Closes. + + What the item-1 PR does. (a) The 6 RED rows were ci.yml sites whose sentences PRQ-013 deleted along with the receipt job (roadmap: "STATED LOSS"), so the rows go. The bats_tests derivation goes too, because no file states that count any more; ci.yml''s fixture_rows site goes from 2 to 1; and scripts/check_pr_review_wiring.sh''s "43-row" joins the table with a self-test row of its own. (b) The :196 comment is reworded so CARGO_RE stops matching: `guard_tree.sh --dry-run --no-cargo` prints `run: scripts/check_pr_review_counts.sh`, and restoring the old wording moves it back to --cargo-only (mutation-verified). (c) The --list mode of mutate-guard.sh and mutate_quorum_arm.sh requires only awk. Before this it demanded bats/jq/minisign/check-jsonschema, so wiring the counts guard into guard-tree, which runs on clean-room HOSTS including the gx10/yoga docker runners, would have turned a dark guard into an ENV-red job (reproduced: rc=2 under a PATH without those four). Sweeps still refuse without them (rc=1). + + Item 2, measured: check_pr_review_receipt.sh HAS a caller with a receipt. check_pr_review_arm4.sh runs `bash "$guard" "$scratch"` (A3) and `bash "$guard" "$best_dir"` (A4) with guard defaulting to it, and pr-review-quorum.yml:141 runs arm4. So there is nothing to fold. What remains is whether guard_tree.sh and the unwired ledger should treat a guard reachable only through another wired guard as wired. That is recorded on the issue and not changed here.' diff --git a/scripts/check_pr_review_counts.sh b/scripts/check_pr_review_counts.sh index d2ba36f814..9955aa41ca 100755 --- a/scripts/check_pr_review_counts.sh +++ b/scripts/check_pr_review_counts.sh @@ -101,15 +101,6 @@ derive_fixture_rows() { printf '%s\n' "$n" } -# bats tests: `@test` at column 0 in the file the CI step actually runs. -derive_bats_tests() { - local root=$1 n - [ -f "$root/tests/pr-review.bats" ] || return 1 - n=$(grep -c '^@test ' "$root/tests/pr-review.bats") - [ "$n" -gt 0 ] || return 1 - printf '%s\n' "$n" -} - # S13 (PRREV-015) adds a SECOND table, a SECOND bats file and a SECOND mutation set. # They are derived here rather than left uncounted, because the universe this guard # iterates is what decides whether it can fail at all: a table it has never heard of is @@ -193,9 +184,11 @@ derive_publish_rows() { # Falsification tests: the contract's own list, counted from its `- id:` labels rather # than from the prose that states the total. It went stale unseen - `pass_criteria` said # 11 over 15 entries at PRREV-013 and over 17 at PRREV-015 - and the only thing that -# noticed was `cargo test -p aprender-contracts --test validate_contracts` printing +# noticed was the aprender-contracts `validate_contracts` integration test printing # `pr-review-skill-v2: pass_criteria says 11 tests, actual 17` into a corpus-wide failure -# nobody reads per-contract. Counting labels and not lines, the same reason S7's rows +# nobody reads per-contract. (Name that test runner's binary in prose here and +# guard_tree.sh's CARGO_RE classifies this whole guard as build-tool-using, which +# drops it from guard-tree's --no-cargo run. That is how it went dark: #3646.) Counting labels and not lines, the same reason S7's rows # carry ids B1..B6. derive_falsification_tests() { local root=$1 n @@ -218,32 +211,43 @@ derive_falsification_tests() { # --------------------------------------------------------------------------- # The site table. id | file | occurrences | template (@N@ -> derived value) +# +# SIX ci.yml ROWS LEFT WITH THE JOB THAT STATED THEM (#3646). PRQ-013 (#2982, #2985) +# deleted ci.yml's receipt job, and with it the step names that carried "233/233", +# "165 tests", "134-mutant" x2, "83 rows" and "Arm 4 case table: 10 rows", plus one +# of the two "43-row" sites. That is the "sentence was deleted" case, and here it was +# deliberate: the roadmap records the sweeps and the fixture table as a STATED LOSS. +# So the rows go, not the sentences. Nobody noticed for weeks because a comment +# naming the build tool had classified this guard out of guard-tree's --no-cargo +# population, so it went RED on main and ran nowhere. The ci.yml rows that remain are +# for step names that still exist. +# +# The bats_tests count went with them. No file states the size of tests/pr-review.bats +# any more, and a derivation with no site checks nothing, so it was removed rather +# than left to read as coverage. If a file states that count again, derive it again +# and add the row. # --------------------------------------------------------------------------- SITES=' mutants|.claude/skills/pr-review/SKILL.md|2|@N@/@N@ mutants|contracts/binding.yaml|1|@N@/@N@ -mutants|.github/workflows/ci.yml|1|@N@/@N@ mutants|docs/specifications/PR-REVIEW-SKILL-002-v2.md|1|@N@/@N@ -fixture_rows|.github/workflows/ci.yml|2|@N@-row +fixture_rows|.github/workflows/ci.yml|1|@N@-row +fixture_rows|scripts/check_pr_review_wiring.sh|1|@N@-row fixture_rows|tests/pr-review.bats|1|@N@ row fixture_rows|tests/pr-review.bats|1|-eq @N@ ] -bats_tests|.github/workflows/ci.yml|2|@N@ tests quorum_mutants|docs/specifications/PR-REVIEW-SKILL-002-v2.md|3|@N@/@N@ quorum_mutants|.claude/skills/pr-review/SKILL.md|1|@N@/@N@ quorum_mutants|contracts/pr-review-skill-v2.yaml|1|@N@/@N@ quorum_mutants|.claude/skills/pr-review/SKILL.md|1|@N@-mutant -quorum_mutants|.github/workflows/ci.yml|2|@N@-mutant quorum_mutants|docs/specifications/PR-REVIEW-SKILL-002-v2.md|2|@N@ mutants quorum_bats_tests|docs/specifications/PR-REVIEW-SKILL-002-v2.md|1|@N@ rows quorum_bats_tests|docs/specifications/PR-REVIEW-SKILL-002-v2.md|1|@N@-row quorum_bats_tests|.claude/skills/pr-review/SKILL.md|2|@N@ rows -quorum_bats_tests|.github/workflows/ci.yml|1|@N@ rows quorum_rows|tests/pr-review-quorum.bats|1|-eq @N@ ] quorum_rows|tests/pr-review-quorum.bats|1|expected @N@ q-* falsification_tests|contracts/pr-review-skill-v2.yaml|1|All @N@ falsification tests shadow_rows|.github/workflows/ci.yml|1|case table: @N@ rows publish_rows|.github/workflows/ci.yml|1|Publisher case table: @N@ rows -arm4_rows|.github/workflows/ci.yml|1|Arm 4 case table: @N@ rows arm4_rows|.github/workflows/pr-review-quorum.yml|1|Arm 4 case table: @N@ rows signer_rows|.github/workflows/ci.yml|1|Signer case table: @N@ rows ' @@ -259,8 +263,6 @@ check() { derived[mutants]=$v v=$(derive_fixture_rows "$root") || die_env "cannot derive the fixture-row count" derived[fixture_rows]=$v - v=$(derive_bats_tests "$root") || die_env "cannot derive the bats test count" - derived[bats_tests]=$v v=$(derive_quorum_mutants "$root") || die_env "cannot derive the S13 mutation-set size (mutate_quorum_arm.sh --list)" derived[quorum_mutants]=$v v=$(derive_quorum_rows "$root") || die_env "cannot derive the S13 q-* fixture-row count" @@ -280,7 +282,7 @@ check() { [ -n "$quiet" ] || { echo "=== the counts these files state as measured, against the tree ($PROG) ===" - echo "derived: mutants=${derived[mutants]} fixture_rows=${derived[fixture_rows]} bats_tests=${derived[bats_tests]}" + echo "derived: mutants=${derived[mutants]} fixture_rows=${derived[fixture_rows]}" echo " quorum_mutants=${derived[quorum_mutants]} quorum_rows=${derived[quorum_rows]} quorum_bats_tests=${derived[quorum_bats_tests]}" echo " falsification_tests=${derived[falsification_tests]} shadow_rows=${derived[shadow_rows]} publish_rows=${derived[publish_rows]} arm4_rows=${derived[arm4_rows]} signer_rows=${derived[signer_rows]}" } @@ -381,7 +383,8 @@ self_test() { scripts/pr_review_quorum_arm.sh \ scripts/pr_review_shadow_record.sh \ scripts/pr_review_shadow_publish.sh \ - scripts/check_pr_review_arm4.sh scripts/pr_review_sign_receipt.sh ) \ + scripts/check_pr_review_arm4.sh scripts/pr_review_sign_receipt.sh \ + scripts/check_pr_review_wiring.sh ) \ | ( cd "$base" && tar -xf - ) || die_env "could not stage a copy of the tree" local nrows=0 @@ -428,8 +431,9 @@ self_test() { sed -i "s#$n/$n#$((n - 1))/$((n - 1))#" "$1/.claude/skills/pr-review/SKILL.md"; } stale_rows() { local n; n=$(derive_fixture_rows "$1") sed -i "s#$n-row#$((n - 1))-row#" "$1/.github/workflows/ci.yml"; } - stale_tests() { local n; n=$(derive_bats_tests "$1") - sed -i "s#$n tests#$((n - 1)) tests#" "$1/.github/workflows/ci.yml"; } + # The site #3646 added: check_pr_review_wiring.sh states the fixture-row count too. + stale_wrows() { local n; n=$(derive_fixture_rows "$1") + sed -i "s#$n-row#$((n - 1))-row#" "$1/scripts/check_pr_review_wiring.sh"; } delete_claim() { local n; n=$(derive_mutants "$1") sed -i "0,\#$n/$n#{\#$n/$n#d}" "$1/contracts/binding.yaml"; } stale_ftests() { local n; n=$(derive_falsification_tests "$1") @@ -440,7 +444,6 @@ self_test() { # let exactly this edit through green. grow_ftests() { sed -i 's#^- id: F-PRREV-002$#- id: F-INVENTED-999\n rule: a falsification test the contract does not count\n\n&#' \ "$1/contracts/pr-review-skill-v2.yaml"; } - grow_bats() { printf '\n@test "a new test the docs do not count" {\n true\n}\n' >> "$1/tests/pr-review.bats"; } grow_rows() { mkdir -p "$1/tests/fixtures/pr-review/row-27-invented"; } grow_mutants() { sed -i 's#^ \[ -n "$head" \] || reject B1 "predicate.head_sha is absent" || return 1#&\n [ -n "$head" ] || reject B1 "an invented rule nothing documents" || return 1#' \ "$1/scripts/check_pr_review_receipt.sh"; } @@ -470,9 +473,10 @@ self_test() { row baseline 0 "the tree as committed" noop row stale-mutation-score 1 "the mutation score written back by one" stale_mutants row stale-fixture-rows 1 "the fixture-row count written back by one" stale_rows - row stale-bats-count 1 "the bats test count written back by one" stale_tests + row stale-wiring-rows 1 "the wiring guard's fixture-row count written back by one" stale_wrows + # stale-bats-count and tree-grew-a-test left with the bats_tests derivation (#3646): + # no file states that count, so neither edit has a claim to falsify. row claim-deleted 1 "the sentence stating the count is deleted" delete_claim - row tree-grew-a-test 1 "a bats test is added and no file says so" grow_bats row tree-grew-a-fixture-row 1 "a row-* fixture is added and no file says so" grow_rows row tree-grew-a-mutant 1 "a reject site is added and no file says so" grow_mutants row stale-quorum-mutation 1 "the S13 mutation score written back by 18" stale_qmutants @@ -505,7 +509,6 @@ case "${1:-}" in --show) printf 'mutants %s\n' "$(derive_mutants "$REPO_ROOT")" printf 'fixture_rows %s\n' "$(derive_fixture_rows "$REPO_ROOT")" - printf 'bats_tests %s\n' "$(derive_bats_tests "$REPO_ROOT")" printf 'quorum_mutants %s\n' "$(derive_quorum_mutants "$REPO_ROOT")" printf 'quorum_rows %s\n' "$(derive_quorum_rows "$REPO_ROOT")" printf 'quorum_bats_tests %s\n' "$(derive_quorum_bats_tests "$REPO_ROOT")" diff --git a/scripts/mutate-guard.sh b/scripts/mutate-guard.sh index 450fc4fced..ad466564b9 100755 --- a/scripts/mutate-guard.sh +++ b/scripts/mutate-guard.sh @@ -104,7 +104,12 @@ while [ "$#" -gt 0 ]; do esac done -for t in bats jq awk minisign check-jsonschema git; do +# --list prints the catalogue and runs no mutant, so it needs only what catalogue() +# reads with. Demanding the sweep's tools there made every count derived from the +# catalogue (check_pr_review_counts.sh) an ENV failure on a runner that cannot run the +# sweep, and guard-tree runs on clean-room hosts, not in a sweep image (#3646). +if [ "$LIST_ONLY" = 1 ]; then need='awk'; else need='bats jq awk minisign check-jsonschema git'; fi +for t in $need; do command -v "$t" >/dev/null 2>&1 || { echo "$PROG: FAIL - $t is not on PATH. A mutation score that could not be measured" >&2 echo " must not be reported as one (S8: guard_mutation_score is a one, not a floor)." >&2 diff --git a/scripts/mutate_quorum_arm.sh b/scripts/mutate_quorum_arm.sh index 995ca8b6da..0139c4de2d 100755 --- a/scripts/mutate_quorum_arm.sh +++ b/scripts/mutate_quorum_arm.sh @@ -72,7 +72,10 @@ while [ "$#" -gt 0 ]; do esac done -for t in bats jq awk minisign check-jsonschema git; do +# --list runs no mutant; it needs only what catalogue() reads with (see +# mutate-guard.sh, #3646). +if [ "$LIST_ONLY" = 1 ]; then need='awk'; else need='bats jq awk minisign check-jsonschema git'; fi +for t in $need; do command -v "$t" >/dev/null 2>&1 || { echo "$PROG: FAIL - $t is not on PATH. A mutation score that could not be measured" >&2 echo " must not be reported as one (S13.10: this number is a one, not a floor)." >&2