Skip to content

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

Merged
shaug merged 4 commits into
mainfrom
claude/101-review-fix-loop-eval-suite
Jul 31, 2026
Merged

feat(review-fix-loop): add the cross-cutting, result-blind evaluation corpus#113
shaug merged 4 commits into
mainfrom
claude/101-review-fix-loop-eval-suite

Conversation

@shaug

@shaug shaug commented Jul 31, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add a reusable, result-blind evaluation corpus for the standalone
    review-fix-loop skill (skills/review-fix-loop/scripts/evals/):
    corpus.py (twenty scenarios), grader.py (independent Git-evidence
    diffing), helpers.py (shared repo/reviewer/decide/apply_fix fixtures,
    importing the identical subset already owned by
    scripts/tests/helpers.py rather than duplicating it), and runner.py
    (the CLI entry point).
  • Cover 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.
  • Grade every scenario against Git evidence computed independently of the
    returned terminal-result document (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) rather than the implementation's own claims.
  • Add scripts/tests/test_evals.py, run under just test: asserts the full
    corpus passes, asserts corpus shape/scope coverage, and demonstrates the
    grader rejecting both a fabricated convergence claim and a fixture that
    cannot actually converge (this ticket's required seeded-fault
    demonstration).
  • Add just test-review-fix-loop and just eval-review-fix-loop, and
    document the corpus in skills/review-fix-loop/evals/README.md and a new
    "Evaluation" section in SKILL.md.

Why

Issue #101 (child of epic #95): establish the standalone review-fix-loop
skill's behavioral contract across both execution modes with deterministic,
result-blind evaluation coverage, complementing (never duplicating) the
capability-owned unit suites test_local_commit.py/test_update_pr.py from
#99/#100.

Acceptance criteria

  • Evaluators are result-blind and cannot pass solely because the
    implementation asserts success — grader.grade_case diffs each
    scenario's own checks mapping against Git evidence independently
    queried via rev_parse/ls_remote/show_file/object_exists/
    rev_list_count/worktree_is_clean/untracked_paths, not by
    re-parsing the terminal-result document. test_evals.py's
    SeededFaultDemonstrationTests demonstrates this directly with two
    tests: one fabricates a "converged" claim with no supporting Git
    evidence and shows it's rejected; one runs the real engine with a
    deliberately unfixable apply_fix fixture and shows a wrong
    "converged" expectation is rejected.
  • The corpus exercises both local_commit and update_pr — 15 lc_*
    scenarios and 5 up_* scenarios; asserted by
    test_corpus_exercises_both_publication_policies.
  • Negative cases prove that incomplete review, reviewer mutation,
    premature publication, lost commits, and stale targets fail closed —
    see the scenario list in skills/review-fix-loop/evals/README.md.
  • Evaluation output identifies the exact fixture, observed evidence, and
    reason for failure — grade_case emits
    f"{case_id}: {name}: expected {expected!r}, observed {observed!r}".
  • The suite is deterministic enough for repository CI — no subprocess
    boundary, no model call, no network; the full corpus ran repeatedly
    with identical results (one genuine timing/content-collision flake was
    found in the two-clone publication-race scenario during development
    and fixed by giving the two clones distinct commit messages).

Validation

  • just format — all checks passed.
  • just lint — all checks passed (all 8 skills validated via skills-ref,
    plugin packaging validated).
  • just test — 721 tests pass, including 273 in
    skills/review-fix-loop/scripts/tests (8 new in test_evals.py) and 318
    in review-suite/scripts/tests.
  • just eval-review-fix-loop (standalone corpus entry point) — 20/20
    scenarios pass, re-run repeatedly with identical results.
  • Two independent fresh-context review-code-change passes: the first found
    one strong_recommendation code-simplicity finding (duplicate fixtures in
    scripts/evals/helpers.py vs. the sibling scripts/tests/helpers.py); the
    fix (importing the identical subset instead of duplicating it) was applied
    and the second pass came back fully clean (solution-simplicity,
    correctness, code-simplicity).

Fixes #101

🤖 Generated with Claude Code

shaug and others added 4 commits July 31, 2026 01:10
… corpus

## Summary
- Add a reusable, result-blind evaluation corpus for the standalone
  `review-fix-loop` skill (`skills/review-fix-loop/scripts/evals/`):
  `corpus.py` (twenty scenarios), `grader.py` (independent Git-evidence
  diffing), `helpers.py` (shared repo/reviewer/decide/apply_fix fixtures),
  and `runner.py` (the CLI entry point).
- Cover 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.
- Grade every scenario against Git evidence computed independently of the
  returned terminal-result document (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) rather than the implementation's own claims.
- Add `scripts/tests/test_evals.py`, run under `just test`: asserts the full
  corpus passes, asserts corpus shape/scope coverage, and demonstrates the
  grader rejecting both a fabricated convergence claim and a fixture that
  cannot actually converge.
- Add `just test-review-fix-loop` and `just eval-review-fix-loop`, and
  document the corpus in `skills/review-fix-loop/evals/README.md` and a new
  "Evaluation" section in `SKILL.md`.

## Why
- Issue #101 (epic #95): establish the standalone skill's behavioral
  contract across both execution modes with deterministic, result-blind
  evaluation coverage, complementing (never duplicating) the capability-owned
  unit suites `test_local_commit.py`/`test_update_pr.py`.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…helpers.py imports

## Summary
- Import `init_repo`, `CLEAN_TEMPLATE`, `ALWAYS_PASS_VALIDATION`, `finding`,
  `make_clean_reviewer`, `fixing_apply_fix`, and `accepting_decide` from the
  sibling `scripts/tests/helpers.py` instead of redefining five
  byte-identical or functionally identical fixtures in
  `scripts/evals/helpers.py`, following this repository's own
  `carve-changesets` precedent of importing across the `scripts/tests`/
  `scripts/evals` boundary within one skill.
- Remove one unused fixture (`make_expanding_findings_reviewer` and its
  `SECOND_FINDING_ID` constant) left over from a descoped scenario.

## Why
- Closes the one strong_recommendation code-simplicity finding the first
  review-code-change pass on #101 found: a second source of truth for
  fixture semantics already owned by `scripts/tests/helpers.py`.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…o's clones

## Summary
- `up_sequential_publication_race_second_clone_loses` clones the canonical
  repo twice (`clone-a`, `clone-b`) but never configured `user.name`/
  `user.email` on either clone. `git clone` never copies a source
  repository's local git config, and unlike a developer machine, a CI runner
  has no global git identity configured either, so the engine's ordinary
  `git commit` (which relies on ambient identity exactly as production code
  does) failed with "Author identity unknown" in GitHub Actions even though
  every local run passed (my own machine's global `~/.gitconfig` masked the
  gap).
- Configure `user.email`/`user.name` on each clone immediately after
  cloning, matching what `helpers.init_repo` already does for every
  non-cloned fixture repo.

## Why
- Fixes PR #113's failing `ci` check (issue #101): reproduced locally with
  `HOME=<empty dir> python3 skills/review-fix-loop/scripts/evals/runner.py`
  (no global git config reachable), confirmed the failure, applied the fix,
  and confirmed all 20 scenarios and all 273
  `skills/review-fix-loop/scripts/tests` tests now pass under the same
  no-global-git-config condition.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
## Summary
- Add the missing CHANGELOG.md entry for commit a44fc2f (configuring git
  identity on the eval publication-race scenario's clones) and backfill the
  full SHA onto the previous entry (81a3078), per AGENTS.md's changelog
  workflow.

## Why
- Closes the one strong_recommendation correctness finding the third
  review-code-change pass on #101 found: the fix commit was missing its
  required changelog entry.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@shaug
shaug merged commit c14c912 into main Jul 31, 2026
1 check passed
@shaug
shaug deleted the claude/101-review-fix-loop-eval-suite branch July 31, 2026 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Build deterministic and result-blind review-fix-loop evaluation coverage

1 participant