feat(review-fix-loop): add the cross-cutting, result-blind evaluation corpus

CHANGELOG.md

@@ -4,15 +4,54 @@ summary: Chronological history of repository and skill changes.
 
 # Changelog
 
-## 2026-07-31 — Recorded the first review-fix-loop `update_pr` fix cycle
+## 2026-07-31 — Added the review-fix-loop cross-cutting evaluation corpus, recorded the first review-fix-loop `update_pr` fix cycle
 
+- fix(review-fix-loop): configure `user.email`/`user.name` on both git clones
+  `scripts/evals/corpus.py`'s
+  `up_sequential_publication_race_second_clone_loses` scenario creates,
+  mirroring `helpers.init_repo`'s existing convention for every non-cloned
+  fixture repo — `git clone` never copies a source repository's local git
+  identity config, and unlike a developer machine a CI runner has no global
+  identity configured either, so the scenario's own `git commit` call failed
+  with "Author identity unknown" in GitHub Actions even though every local run
+  passed; reproduced the exact CI failure locally under a forced no-identity
+  condition, confirmed the fix resolves it, and confirmed GitHub Actions' own
+  `ci` check on PR #113 is green
+- fix(review-fix-loop): consolidate `scripts/evals/helpers.py`'s five fixtures
+  that were byte-identical or functionally identical to
+  `scripts/tests/helpers.py`'s own (`init_repo`, `CLEAN_TEMPLATE`,
+  `ALWAYS_PASS_VALIDATION`, `finding`, `make_clean_reviewer`,
+  `fixing_apply_fix`, `accepting_decide`) into imports from that sibling module
+  instead of a second source of truth, following this repository's own
+  `carve-changesets` precedent of importing across the `scripts/tests`/
+  `scripts/evals` boundary within one skill, and remove one unused reviewer
+  fixture (`make_expanding_findings_reviewer`) left over from a descoped
+  scenario, closing the one code-simplicity gap the first review-code-change
+  pass on #101 found (`81a3078c819d4bc8755a9a796d2fa4c0e7dbf1c4`)
+- feat(review-fix-loop): add the cross-cutting, result-blind evaluation corpus
+  (issue #101, epic #95) covering convergence, repeated findings,
+  invalid/incomplete reviews, declined findings, budget exhaustion, interruption
+  and recovery, validation failure, reviewer mutation, and publication races
+  across both `local_commit` and `update_pr`, plus the fresh-subagent default
+  and the explicit in-agent override — twenty scenarios in
+  `scripts/evals/corpus.py`, each driving the real engine against a real
+  disposable Git repository (and, for `update_pr`, a real disposable bare
+  remote) and graded in `scripts/evals/grader.py` against independently derived
+  Git evidence (a real commit count, a real file's content at a real commit, a
+  real remote ref, a real object's reachability) rather than the returned
+  terminal-result document's own claims; `scripts/tests/test_evals.py`
+  demonstrates the grader rejecting both a fabricated convergence claim and a
+  fixture that cannot actually converge, and runs the whole corpus under
+  `just test`; `just eval-review-fix-loop` is the standalone entry point
+  (`cd5b3ee63d89fed305c4e5a3c0f15cb14b84a3c6`)
 - fix(review-fix-loop): extract the test fixtures shared between
   `test_local_commit.py` and `test_update_pr.py` (the module loader, a bare
   local repository, the always-passing validation commands, the
   marker-file-driven fake reviewer, and the accepting decider/fixer) into a
   sibling `scripts/tests/helpers.py`, matching
   `carve-changesets/scripts/tests/helpers.py`'s established precedent, closing
   the one code-simplicity gap the first review-code-change pass on #100 found
+  (`729135bb11d5bd8f0efa3a66d1c1ab1f978a3f6d`)
 
 ## 2026-07-30 — Delivered and evaluated the standalone review-fix-loop `update_pr` workflow, delivered and evaluated the standalone review-fix-loop `local_commit` workflow, implemented the review-fix-loop reviewer isolation and complete-review orchestration and local execution substrate (common-directory locking, isolated attempts, checkpoint persistence, and recovery), defined the review-fix-loop invocation, checkpoint, and terminal-result contracts, removed the unproven verification-sufficiency pass and its required-evidence field from review-correctness, and simplified the review-fix-loop design around local coordination and Git-native publication safety
 

justfile

@@ -110,6 +110,18 @@ test-implement-epic:
 test-carve-changesets:
   python3 -m unittest discover -s {{skills_dir}}/carve-changesets/scripts/tests -p 'test_*.py'
 
+test-review-fix-loop:
+  python3 -m unittest discover -s {{skills_dir}}/review-fix-loop/scripts/tests -p 'test_*.py'
+
+# Result-blind, deterministic replay of the review-fix-loop cross-cutting
+# corpus: drives the real local_commit/update_pr engine against disposable
+# Git repositories with scripted reviewer/decide/apply_fix fixtures (no
+# subprocess boundary, no model call, no network). Also exercised under
+# `just test` via scripts/tests/test_evals.py; this target is the standalone
+# entry point for ad hoc runs and per-scenario `--output-dir` reports.
+eval-review-fix-loop:
+  python3 {{skills_dir}}/review-fix-loop/scripts/evals/runner.py
+
 eval-carve-changesets:
   python3 {{skills_dir}}/carve-changesets/scripts/evals/runner.py --integration-self-test
   python3 {{skills_dir}}/carve-changesets/scripts/evals/runner.py

skills/review-fix-loop/SKILL.md

@@ -12,7 +12,7 @@ review suite, applies material ticket-scoped fixes, and repeats until review
 converges or a bounded stop condition is reached. The full design is
 [`design/review-fix-loop.md`](../../design/review-fix-loop.md).
 
-Five of its children are implemented so far:
+Six of its children are implemented so far:
 
 - [Issue #96](https://github.com/shaug/agent-scripts/issues/96) (the first of
   epic [#95](https://github.com/shaug/agent-scripts/issues/95)) defines and
@@ -52,6 +52,11 @@ Five of its children are implemented so far:
   publication race is lost or the remote is unavailable. See
   [Run the standalone `update_pr` workflow](#run-the-standalone-update_pr-workflow)
   below.
+- [Issue #101](https://github.com/shaug/agent-scripts/issues/101) adds the
+  cross-cutting, result-blind evaluation corpus that establishes this skill's
+  behavioral contract across both execution modes from externally observable Git
+  evidence, not the returned terminal-result document's own claims. See
+  [Evaluation](#evaluation) below.
 
 Use [`scripts/local_commit.py`](scripts/local_commit.py)'s
 `run_local_commit(...)` to run a complete standalone `local_commit` invocation
@@ -275,6 +280,24 @@ unreachable remote, the remote-target lock actually being exercised through
 `run_update_pr`, and rejection of an invalid invocation or a `local_commit`
 invocation at the API boundary.
 
+## Evaluation
+
+[`evals/README.md`](evals/README.md) documents the cross-cutting, result-blind
+evaluation corpus in [`scripts/evals/`](scripts/evals): twenty scenarios,
+covering convergence, repeated findings, invalid/incomplete reviews, declined
+findings, budget exhaustion, interruption and recovery, validation failure,
+reviewer mutation, and publication races across both `local_commit` and
+`update_pr`, plus the fresh-subagent default and the explicit in-agent override.
+Every scenario drives the real engine against a real disposable Git repository
+(and, for `update_pr`, a real disposable bare remote) and is graded against
+independently derived Git evidence — a real commit count, a real file's content
+at a real commit, a real remote ref, a real object's reachability — never the
+returned terminal-result document's own claims, so a result that only *asserts*
+success cannot pass. No subprocess boundary and no model call is involved, so
+the whole corpus runs deterministically under `just test` (via
+[`scripts/tests/test_evals.py`](scripts/tests/test_evals.py)) and standalone via
+`just eval-review-fix-loop`.
+
 ## Non-goals
 
 - Creating, merging, closing, or otherwise managing a pull request.

skills/review-fix-loop/evals/README.md

@@ -0,0 +1,71 @@
+# Review-fix-loop evaluations
+
+Issue #101's cross-cutting, result-blind evaluation corpus for the standalone
+`review-fix-loop` skill. It does not replace
+`scripts/tests/test_local_commit.py` or `scripts/tests/test_update_pr.py` —
+those remain the capability-owned unit suites for issues #99/#100. This corpus
+instead proves the skill's whole behavioral contract across both publication
+policies, from externally observable Git evidence rather than the returned
+terminal-result document's own claims.
+
+## Where the corpus lives
+
+Every other evaluated skill in this repository (`carve-changesets`,
+`implement-ticket`, `babysit-pr`) keeps its scenario data in this directory as
+`cases.json`/`expectations.json`, because each of their scenarios is a short
+natural-language situation description handed to an agent-shaped executor for
+judgment. `review-fix-loop` has no equivalent judgment surface to evaluate that
+way: its actual behavior is the deterministic Python engine in
+`../scripts/local_commit.py` and `../scripts/update_pr.py`, and the three
+genuinely host-boundary actions (`reviewer`, `decide`, `apply_fix`) are already
+exercised with scripted fixtures by the capability-owned unit suites. A JSON
+case format here would either duplicate that fixture plumbing or describe
+scenarios too shallowly to build the real Git repositories, disposable bare
+remotes, and interleaved-clone races several scenarios require.
+
+The corpus is therefore a registry of plain Python scenario functions in
+`../scripts/evals/corpus.py` (`CORPUS.SCENARIOS`/`CORPUS.SCENARIOS_BY_ID`), each
+of which drives the real engine against a real temporary repository and reports
+a `checks` mapping of independently-derived Git evidence for
+`../scripts/evals/grader.py` to diff. This directory still exists, per the
+repository's convention that every evaluated skill keeps its evaluation
+documentation and data under `evals/`, and is the place to look for what this
+corpus asserts and why.
+
+## Running it
+
+```bash
+just eval-review-fix-loop            # run the whole corpus once
+python3 scripts/evals/runner.py --list                 # list scenario ids
+python3 scripts/evals/runner.py --scenario lc_converged_after_one_fix
+python3 scripts/evals/runner.py --output-dir /tmp/out   # per-scenario JSON reports
+```
+
+No subprocess boundary and no model call is involved: every scenario is an
+in-process, deterministic replay, so the whole corpus is free and safe to run in
+repository CI. `scripts/tests/test_evals.py` already runs it under `just test`;
+`eval-review-fix-loop` is the standalone entry point for ad hoc runs and
+per-scenario reports.
+
+## Scope
+
+The corpus mirrors this ticket's own "Scope" bullet list: convergence, repeated
+findings, invalid reviews, declined findings, budget exhaustion, interruption,
+recovery, validation failure, reviewer mutation, and publication races — across
+both `local_commit` and `update_pr` — plus fresh-subagent defaults and the
+explicit in-agent override. It deliberately does not add dedicated
+`oscillation`/`repeated_failed_attempt` scenarios: those `changes_remaining`
+reasons are already covered by `scripts/tests/test_local_commit.py`.
+
+## Result-blindness
+
+`grader.grade_case` never trusts a scenario's returned terminal-result document
+by itself. Each scenario's `checks` mapping compares the terminal contract's own
+declared fields (`terminal_state`, `reason`) *and* a set of facts computed
+independently by asking Git directly — a real commit count, a real file's
+content at a real commit, a real remote ref, a real object's reachability, a
+real dirty-worktree listing — so a result that lies about what happened
+disagrees with the evidence rather than being trusted.
+`scripts/tests/test_evals.py`'s `SeededFaultDemonstrationTests` demonstrates
+this directly: a fabricated "converged" claim with no matching Git evidence, and
+a deliberately unfixable fixture declared to converge, are both rejected.