Skip to content

feat(reviewer-eval): N-arm matrix, blinded grading, corpus 2->11 for the GPT-5.6 evaluation#700

Merged
igerber merged 1 commit into
mainfrom
ai-review-5-6
Jul 19, 2026
Merged

feat(reviewer-eval): N-arm matrix, blinded grading, corpus 2->11 for the GPT-5.6 evaluation#700
igerber merged 1 commit into
mainfrom
ai-review-5-6

Conversation

@igerber

@igerber igerber commented Jul 18, 2026

Copy link
Copy Markdown
Owner

Summary

  • N-arm eval matrix: config/configs.json moves from the two-arm control/candidate shape to an arms list (exactly one role: control) plus declared treatment_fields; loading is fail-closed (duplicate ids, unknown per-arm keys, missing model/effort, zero-or-multiple controls all abort). Current config: the 4-arm gpt-5.6 evaluation (gpt-5.5@xhigh control, gpt-5.6-sol@xhigh, gpt-5.6-terra@xhigh, gpt-5.6-sol@max), cli pin bumped to codex-cli 0.144.5 (verified live 2026-07-18).
  • Declared-treatment confound rules (engine/runner.py): undeclared confounds (incl. model) must be identical across arms; no two arms may share a treatment tuple; every arm needs a single-field contrast partner (a jointly-confounded subset like model+effort with no bridging arm is refused). Multi-arm only - single-arm smokes stay exempt.
  • Per-config repeats: run --k-per "C=1,D=1" (fail-closed parse) inside ONE invocation/manifest; k/k_per recorded in the success manifest.
  • Blinded grading: compare --blinded writes comparison.blinded.md + a sealed blinding.json - arm ids remapped to M* labels via a permutation salted from the manifest's run_ids, model/CLI/latency metadata redacted, model self-references scrubbed (configured names + gpt-family/tier/bare-version patterns) from reviews AND case notes; refuses --allow-mixed / manifest-less subdirs.
  • Effort + timeout plumb-through (.claude/scripts/openai_review.py): _build_codex_cmd/call_codex gain effort= (fail-closed allowlist; ultra deliberately excluded - not accepted on this API path) and timeout_s= with byte-identical production defaults; stdin is fed off-thread with output drains started first, so the timeout is armed immediately and a full-pipe write cannot deadlock; the eval adapter runs SUPPORTED_EFFORTS = ("xhigh", "max") with a 60-min per-run ceiling (hung run -> resumable INFRA_ERROR).
  • Corpus 2 -> 11 verified cases: +1 S1 synthetic revert (CS no-covariate DR per-cell SE plug-in vs returned IF, PR fix(callaway): IF-based per-cell SE for no-covariate doubly-robust #627 fix), +4 S2 historical replays of reviewed rounds where a real P0/P1 was caught (fix(inference): three standard-error parity fixes vs canonical R #620 non-finite BM-DOF fail-open, feat(had): cluster-robust SE on the continuous paths (Phase 2a) #596 missing cluster-count guard, feat(continuous-did): discrete-treatment saturated regression (treatment_type="discrete") #618 discrete-ACRT=0 estimand, feat: dedicated practitioner_next_steps() handler for ChangesInChanges/QDiD #692 CiC/QDiD practitioner screen framing), +1 S3 documented-deviation negative control (comment-only diff over the WCR p-floor; allow_severities: ["P3"] probes severity calibration), +3 S4 missed-bug probes replaying rounds the reviewer passed while a later-flagged P1 was already present (Add LPDiD complex-survey-design support (Phase D1) #590, feat(callaway): allow_unbalanced_panel — RC-on-panel parity with R did #631, Fix wild cluster bootstrap p-value / CI inconsistency (WCR + test inversion, #543) #546 - the last with two latent P1s and a tutorial notebook exercising the notebook-prose path).
  • Pre-registered decision rule: tools/reviewer-eval/DECISION_RULE.md commits the GO/NO-GO gates, blinded multi-grader protocol, and corpus floor BEFORE any campaign run - this PR timestamps the pre-registration.

Methodology references (required if estimator / math changes)

  • Method name(s): N/A - no live estimator/methodology changes. Estimator-looking diffs are frozen eval fixtures under tools/reviewer-eval/corpus/cases/**/inject.diff (historical PR replays / deliberate bug injections with documented ground truth); they are corpus data, not library changes.
  • Paper / source link(s): N/A
  • Any intentional deviations from the source (and why): None

Validation

  • Tests added/updated: tests/test_evals_runtime.py (+18: fail-closed configs/treatments incl. model-held-constant regression, k-per parse + end-to-end, blinded compare refusals/determinism), tests/test_evals_engine.py (+6: blind mapping determinism, sanitizer, no-identity-leak bundle, allow_severities-derived FP wording), tests/test_openai_review.py (+6: effort argv byte-identity/passthrough/invalid, timeout kill, blocking-stdin timeout coverage). CLI-pin literals now read from configs.json. Suites: 111 eval + 260 openai_review tests green; verify-corpus 11/11.
  • Backtest / simulation / notebook evidence (if applicable): N/A (the eval campaign itself runs locally after merge-independent review; runs/ is gitignored)

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

Generated with Claude Code

https://claude.ai/code/session_019xnguzh8asX9dY54djvR41

…the gpt-5.6 evaluation

Harness: configs.json arms list + declared treatment_fields (fail-closed
loading); runner single-field-contrast confound rules; --k-per per-config
repeats; compare --blinded (deterministic sealed mapping, model-ref
sanitization, latency redaction); effort + timeout_s plumb-through in
openai_review.py with byte-identical production defaults.

Corpus: 2 -> 11 cases (2 S1 synthetic, 4 S2 historical replays, 2 S3
negative controls incl. a documented-deviation calibration probe, 3 S4
missed-bug probes). Pre-registered GO/NO-GO gates in DECISION_RULE.md.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019xnguzh8asX9dY54djvR41
@github-actions

Copy link
Copy Markdown

Overall Assessment

Looks good — no unmitigated P0/P1 findings.

Executive Summary

  • No live estimator, weighting, variance/SE, or identification code is changed; estimator-looking diffs are frozen reviewer-eval corpus fixtures.
  • N-arm config loading, declared-treatment validation, duplicate/unknown config fail-closed behavior, and single-field contrast checks are covered in code and tests.
  • Blinded compare redacts model/effort/latency metadata and refuses manifest-less or mixed blinded grading.
  • Corpus validation is defensive around duplicate IDs, expected files for negative controls, exact post-diff path matching, and stored-patch containment.
  • I did not run tests in this read-only sandbox; assessment is from code/test inspection.

Methodology

No findings.

The PR does not modify live diff_diff/*.py estimator behavior. The methodology-relevant diffs are corpus inject.diff fixtures, with ground truth encoded in case metadata. The WCR p-floor negative control correctly targets a documented REGISTRY.md deviation, not a defect: docs/methodology/REGISTRY.md:L142-L151, tools/reviewer-eval/corpus/cases/s3_negative/s3-wcr-pfloor-comment/case.json:L1-L28.

The ContinuousDiD ACRT fixture is consistent with the documented binary/single-dose boundary: docs/methodology/continuous-did.md:L195-L225, tools/reviewer-eval/corpus/cases/s2_historical/s2-acrt-single-dose/case.json:L17-L36.

Code Quality

No findings.

Key fail-closed paths are implemented in tools/reviewer-eval/run_eval.py:L96-L226, tools/reviewer-eval/engine/runner.py:L138-L176, and tools/reviewer-eval/adapters/codex_reviewer.py:L241-L289.

Performance

No findings.

The new timeout handling prevents hung Codex runs from blocking an eval campaign, and stdin feeding/output draining avoid full-pipe deadlock: .claude/scripts/openai_review.py:L1418-L1467.

Maintainability

No findings.

Experiment identity now includes model, effort, action version, CLI, prompt, and backend invocation contract, reducing stale-cache ambiguity: tools/reviewer-eval/adapters/codex_reviewer.py:L99-L151.

Tech Debt

No findings.

Existing deferred methodology limitations relevant to added missed-bug cases are already tracked in TODO.md, including LPDiD survey gaps and CallawaySantAnna unbalanced-panel follow-ups: TODO.md:L104-L122.

Security

No findings.

Subdir traversal, stored-patch containment, and worktree cleanup are guarded: tools/reviewer-eval/run_eval.py:L62-L68, tools/reviewer-eval/adapters/worktree.py:L73-L90, tools/reviewer-eval/adapters/worktree.py:L212-L225.

Documentation/Tests

No findings.

The added tests cover N-arm config validation, treatment-field rules, per-config repeats, blinded compare refusals/determinism, effort passthrough, and timeout behavior: tests/test_evals_runtime.py:L1679-L2100, tests/test_evals_engine.py:L193-L296, tests/test_openai_review.py:L2845-L3048.

@igerber igerber added the ready-for-ci Triggers CI test workflows label Jul 18, 2026
@igerber
igerber merged commit 96e5667 into main Jul 19, 2026
28 of 29 checks passed
@igerber
igerber deleted the ai-review-5-6 branch July 19, 2026 00:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-ci Triggers CI test workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant