diff --git a/.claude/commands/ai-review-local.md b/.claude/commands/ai-review-local.md index 198648835..7ca427c66 100644 --- a/.claude/commands/ai-review-local.md +++ b/.claude/commands/ai-review-local.md @@ -16,7 +16,7 @@ Two backends are supported: | Backend | Latency | Cost | Quality | |---|---|---|---| -| `api` (`gpt-5.5`) | 30-60s | ~$0.10-1.00/run, metered via `OPENAI_API_KEY` | Single-shot — won't grep, can't load files on its own initiative | +| `api` (`gpt-5.6-sol`) | 30-60s | ~$0.10-1.00/run, metered via `OPENAI_API_KEY` | Single-shot — won't grep, can't load files on its own initiative | | `codex` (any auth) | 3-15 min | depends on your `codex login` mode (subscription vs API key) — see codex docs | Agentic — matches CI Codex reviewer, can grep / load files / multi-turn | Choose with `--backend {auto,codex,api}` (default `auto`): @@ -66,8 +66,8 @@ Notes: *Api backend only.* - `--force-fresh`: Skip delta-diff mode, run a full fresh review even if previous state exists - `--full-registry`: Include the entire REGISTRY.md instead of selective sections -- `--model `: Override the model (default: `gpt-5.5`). Applies to both backends. -- `--timeout `: HTTP request timeout. If omitted, defaults to 900 for reasoning models (gpt-5.4/gpt-5.5, *-pro, o1/o3/o4) and 300 otherwise. *Api backend only.* +- `--model `: Override the model (default: `gpt-5.6-sol`). Applies to both backends. +- `--timeout `: HTTP request timeout. If omitted, defaults to 900 for reasoning models (gpt-5.4/gpt-5.5/gpt-5.6*, *-pro, o1/o3/o4) and 300 otherwise. *Api backend only.* - `--dry-run`: Print the compiled prompt without invoking the chosen backend (no API call, no codex subprocess) @@ -107,7 +107,7 @@ Step 5 invokes the chosen backend: Parse `$ARGUMENTS` for the optional flags listed above. All flags are optional — the default behavior (auto-detect backend, standard context for api or -agentic loading for codex, selective registry, gpt-5.5) +agentic loading for codex, selective registry, gpt-5.6-sol) requires no arguments. ### Step 2: Validate Prerequisites @@ -417,8 +417,8 @@ would be silently ignored. Note: `--force-fresh` is a skill-only flag — it controls whether delta diffs are generated in Step 4 and is NOT passed to the script. -**Reasoning model handling:** If the model is `gpt-5.4`/`gpt-5.5`, contains `-pro`, or starts with -`o1`/`o3`/`o4` (e.g., `gpt-5.5`, `gpt-5.4-pro`, `o3`, `o4-mini`): +**Reasoning model handling:** If the model starts with `gpt-5.4`/`gpt-5.5`/`gpt-5.6`, contains `-pro`, or starts with +`o1`/`o3`/`o4` (e.g., `gpt-5.6-sol`, `gpt-5.4-pro`, `o3`, `o4-mini`): - The script auto-resolves `--timeout` to 900s for reasoning models when omitted, so no extra flag is required unless overriding - Run the Bash command with `run_in_background: true` (bypasses the 600s Bash tool timeout cap) diff --git a/.claude/scripts/openai_review.py b/.claude/scripts/openai_review.py index 2f9eb82b5..5164548cd 100644 --- a/.claude/scripts/openai_review.py +++ b/.claude/scripts/openai_review.py @@ -870,6 +870,9 @@ def apply_token_budget( "gpt-5.4-pro": (30.00, 180.00), "gpt-5.5": (5.00, 30.00), "gpt-5.5-pro": (30.00, 180.00), + "gpt-5.6-sol": (5.00, 30.00), + "gpt-5.6-terra": (2.50, 15.00), + "gpt-5.6-luna": (1.00, 6.00), "gpt-4.1": (2.00, 8.00), "gpt-4.1-mini": (0.40, 1.60), "o3": (2.00, 8.00), @@ -1133,7 +1136,7 @@ def compile_prompt( # --------------------------------------------------------------------------- ENDPOINT = "https://api.openai.com/v1/responses" -DEFAULT_MODEL = "gpt-5.5" +DEFAULT_MODEL = "gpt-5.6-sol" DEFAULT_TIMEOUT = 300 # seconds REASONING_TIMEOUT = 900 # seconds DEFAULT_MAX_TOKENS = 16384 @@ -1142,13 +1145,13 @@ def compile_prompt( def _is_reasoning_model(model: str) -> bool: """Return True for models that use internal chain-of-thought reasoning.""" - return model.startswith(("o1", "o3", "o4", "gpt-5.4", "gpt-5.5")) or "-pro" in model + return model.startswith(("o1", "o3", "o4", "gpt-5.4", "gpt-5.5", "gpt-5.6")) or "-pro" in model def _resolve_timeout(timeout: "int | None", model: str) -> int: """Auto-resolve omitted --timeout based on model class. - Reasoning models (o1/o3/o4/gpt-5.4/gpt-5.5/*-pro) get REASONING_TIMEOUT (900s). + Reasoning models (o1/o3/o4/gpt-5.4/gpt-5.5/gpt-5.6*/*-pro) get REASONING_TIMEOUT (900s). Non-reasoning models get DEFAULT_TIMEOUT (300s). Explicit values are passed through unchanged. """ @@ -1333,7 +1336,7 @@ def _build_codex_cmd( ) -> "list[str]": """Construct the argv for `codex exec`. - Pinned to match the CI Codex action's invocation (gpt-5.5 + xhigh effort + + Pinned to match the CI Codex action's invocation (gpt-5.6-sol + xhigh effort + read-only sandbox) so local reviews give CI-equivalent quality. ``effort`` defaults to the production/CI value (xhigh) so this argv stays byte-identical for production callers; the reviewer-eval harness passes other levels @@ -1745,7 +1748,7 @@ def main() -> None: default=None, help=( f"HTTP request timeout in seconds (api backend only). If omitted, " - f"defaults to {REASONING_TIMEOUT} for reasoning models (gpt-5.4/gpt-5.5, " + f"defaults to {REASONING_TIMEOUT} for reasoning models (gpt-5.4/gpt-5.5/gpt-5.6*, " f"*-pro, o1/o3/o4) and {DEFAULT_TIMEOUT} otherwise. Ignored under " f"--backend codex (codex manages its own time budget)." ), diff --git a/.github/workflows/ai_pr_review.yml b/.github/workflows/ai_pr_review.yml index 344c12db2..5e6f89a7f 100644 --- a/.github/workflows/ai_pr_review.yml +++ b/.github/workflows/ai_pr_review.yml @@ -520,7 +520,7 @@ jobs: sandbox: read-only safety-strategy: drop-sudo # Recommended by OpenAI for review quality/consistency: - model: gpt-5.5 + model: gpt-5.6-sol effort: xhigh - name: Post PR comment (new on every event except initial open) diff --git a/CHANGELOG.md b/CHANGELOG.md index 27843b4d2..cd40dddeb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed +- **CI + local AI PR-reviewer model upgraded `gpt-5.5` -> `gpt-5.6-sol` (effort stays + `xhigh`).** Validated empirically before the swap via the `tools/reviewer-eval/` + 4-arm blinded campaign (66 runs over 11 cases; three independent identity-blind + graders; pre-registered gates in `DECISION_RULE.md`): gpt-5.6-sol@xhigh reliably + caught 7/10 ground-truth bugs vs gpt-5.5@xhigh's 2/10 (+2 unstable), with zero + catch regressions and three strict improvements including an S4 missed-bug probe; + gpt-5.6-sol@max showed no decisive gain over xhigh at ~1.5-2x latency (default + stays xhigh); gpt-5.6-terra tracked ~gpt-5.5. Touched pins: `ai_pr_review.yml` + `model:`, `openai_review.py` `DEFAULT_MODEL` / `_is_reasoning_model` / `PRICING` + (+gpt-5.6-sol 5/30, -terra 2.50/15, -luna 1/6 per 1M), `ai-review-local.md` doc + refs, and `configs.json` (control arm flipped to gpt-5.6-sol@xhigh; gpt-5.5 + retained as the previous-production arm). Also fixes the `s3-wcr-pfloor-comment` + corpus fixture: the campaign's winning arms correctly flagged a comment overclaim + the fixture itself introduced ("an exact zero is never reported" vs the + floor >= alpha branch returning the raw p-value) - the reworded comment now uses + purpose-phrasing, restoring the case as a genuinely clean negative control. + ### Added - **Reviewer-eval harness: N-arm matrix, blinded grading, corpus grown 2 -> 11 (`tools/reviewer-eval/`, prep for the GPT-5.6 reviewer evaluation).** diff --git a/tests/test_evals_runtime.py b/tests/test_evals_runtime.py index 43ab6b913..efc3bdd17 100644 --- a/tests/test_evals_runtime.py +++ b/tests/test_evals_runtime.py @@ -2098,3 +2098,43 @@ def test_run_matrix_holds_model_constant_when_not_a_treatment(monkeypatch): ] with pytest.raises(ConfoundMismatch): run_matrix([_case()], cfgs, r, store, k=1, max_parallel=1, treatment_fields=("effort",)) + + +def test_smoke_default_selects_control_arm(tmp_path, monkeypatch): + """Bare `smoke` (no --configs) must exercise the sole role=control arm from + configs.json - a control flip must never leave the default smoking a stale + arm (local review P2 on the gpt-5.6 swap).""" + import json as _json + + import run_eval + from engine.models import STRATUM_SYNTHETIC, Case + + monkeypatch.setattr(run_eval, "RUNS_DIR", str(tmp_path / "runs")) + monkeypatch.setattr( + run_eval.CorpusLoader, + "load_cases", + lambda self, strata: [Case(id="c", stratum=STRATUM_SYNTHETIC)], + raising=True, + ) + monkeypatch.setattr(run_eval.CorpusLoader, "verify", lambda self, case: None, raising=True) + + class _Rev: + def cli_version(self): + return _pinned_cli_version() + + monkeypatch.setattr(run_eval, "_build_reviewer", lambda repo_root: _Rev(), raising=True) + + captured = {} + + def _capture_run_matrix(cases, configs, *a, **k): + captured["config_ids"] = [c.id for c in configs] + return [] + + monkeypatch.setattr("engine.runner.run_matrix", _capture_run_matrix, raising=True) + rc = run_eval.cmd_smoke(_ns(configs=None, strata=None, k=1, limit=1, max_parallel=2)) + assert rc == 0 + + live = _json.loads((_EVAL_ROOT / "config" / "configs.json").read_text()) + control = [c["id"] for c in live["arms"] if c.get("role") == "control"] + assert captured["config_ids"] == control, "bare smoke must run exactly the control arm" + assert run_eval._control_id() == control[0] diff --git a/tests/test_openai_review.py b/tests/test_openai_review.py index 1dfeb1c77..235fa8133 100644 --- a/tests/test_openai_review.py +++ b/tests/test_openai_review.py @@ -1653,6 +1653,26 @@ def test_rejects_traversal(self, review_mod, repo_root): class TestIsReasoningModel: + def test_gpt56_family_is_reasoning(self, review_mod): + """The production default (gpt-5.6-sol) and its siblings must classify as + reasoning models - this drives the 900s timeout and token limits.""" + for m in ("gpt-5.6-sol", "gpt-5.6-terra", "gpt-5.6-luna"): + assert review_mod._is_reasoning_model(m) is True + + def test_gpt56_sol_timeout_resolves_to_reasoning(self, review_mod): + assert review_mod._resolve_timeout(None, "gpt-5.6-sol") == review_mod.REASONING_TIMEOUT + + def test_gpt56_pricing_entries_present(self, review_mod): + """The three GPT-5.6 tiers priced per the 2026-07 OpenAI table (per 1M).""" + assert review_mod.PRICING["gpt-5.6-sol"] == (5.00, 30.00) + assert review_mod.PRICING["gpt-5.6-terra"] == (2.50, 15.00) + assert review_mod.PRICING["gpt-5.6-luna"] == (1.00, 6.00) + + def test_default_model_is_gpt56_sol(self, review_mod): + """DEFAULT_MODEL is the production reviewer pin (kept in lockstep with the + CI workflow's model: input; the eval harness validated this pairing).""" + assert review_mod.DEFAULT_MODEL == "gpt-5.6-sol" + def test_o3_is_reasoning(self, review_mod): assert review_mod._is_reasoning_model("o3") is True diff --git a/tools/reviewer-eval/README.md b/tools/reviewer-eval/README.md index 1965ee37b..fee1e6106 100644 --- a/tools/reviewer-eval/README.md +++ b/tools/reviewer-eval/README.md @@ -36,9 +36,10 @@ tools/reviewer-eval/ # 1. Verify the corpus materializes (no codex; fast) python tools/reviewer-eval/run_eval.py verify-corpus -# 2. Smoke test (1 case, control arm, first real codex call); smoke each -# candidate arm too — `smoke --configs D` live-proves the max effort level -python tools/reviewer-eval/run_eval.py smoke --configs A +# 2. Smoke test (1 case, first real codex call). Bare `smoke` exercises the +# role=control arm (current production, derived from configs.json); smoke +# each candidate arm too — `smoke --configs D` live-proves the max effort +python tools/reviewer-eval/run_eval.py smoke # 3. Full matrix — k=2 repeats on the primary A/B, single-shot probe arms python tools/reviewer-eval/run_eval.py run --subdir gpt56 --configs A,B,C,D --k 2 --k-per C=1,D=1 diff --git a/tools/reviewer-eval/campaigns/2026-07-gpt56-VERDICT.md b/tools/reviewer-eval/campaigns/2026-07-gpt56-VERDICT.md new file mode 100644 index 000000000..42bd083e4 --- /dev/null +++ b/tools/reviewer-eval/campaigns/2026-07-gpt56-VERDICT.md @@ -0,0 +1,91 @@ +# GPT-5.6 Reviewer Evaluation - Verdict (2026-07-18) + +Campaign: 66/66 runs ok, 11 cases, arms A=gpt-5.5@xhigh k2 (control), B=gpt-5.6-sol@xhigh +k2, C=gpt-5.6-terra@xhigh k1, D=gpt-5.6-sol@max k1. Blinded grading: 3 independent +graders on comparison.blinded.md (labels M1-M4; graders never saw blinding.json); +near-perfect inter-grader agreement (single gate-irrelevant cell disagreement: +s2-acrt/C/r0 graded missed-missed-partial; partial counts as missed). Unblinded only +after tables were final. Mapping: A=M4, B=M3, C=M2, D=M1. + +## Catch table (10 ground-truth bugs; "reliable" = caught in ALL of that arm's repeats) + +| Bug (stratum) | A 5.5@xh r0/r1 | B sol@xh r0/r1 | C terra r0 | D sol@max r0 | +|---|---|---|---|---| +| s1-coef-dict-collision (S1) | C / C | C / C | C | C | +| s1-cs-dr-plugin-se (S1) | C / C | C / C | C | C | +| s2-acrt-single-dose (S2, hist. caught) | M / C | C / C | M | C | +| s2-cic-practitioner-screen (S2) | M / M | C / C | C | C | +| s2-had-cluster-guard (S2) | P / M | C / C | M | C | +| s2-nan-dof-fail-closed (S2) | M / M | C / P | P | C | +| s4-cs-unbalanced-nan-outcome (S4) | M / M | C / C | M | C | +| s4-lpdid-survey-unreduced-design (S4) | M / C | C / C | P | P | +| s4-wcr-saturated-guard-order:b1 (S4) | M / M | M / M | M | M | +| s4-wcr-saturated-guard-order:b2 (S4) | M / M | M / M | M | M | +| **Reliable catches** | **2/10** (+2 unstable) | **7/10** (+1 unstable) | **3/10** | **7/10** | + +C=caught, P=partial (counts as missed for gates), M=missed. + +## Pre-registered gates (B vs A) + +1. **Regression gate: PASS.** A reliably catches only the two S1 reverts; B catches both + 2/2. Zero must_catch bugs where A reliable + B missed-both. (A could not even + reproduce its own historical S2 catches under the current prompt: s2-had/s2-nan-dof/ + s2-cic missed in both repeats, s2-acrt in one.) +2. **FP gate: MECHANICAL FAIL, INVALID CONTROL.** B posted 1 "FP" (P2, s3-wcr-pfloor + r0) vs A's 0. Post-verdict code inspection confirms the flagged finding is FACTUALLY + CORRECT: the fixture's reworded comment claims "an exact zero is never reported" + while the floor>=alpha branch returns raw_p, which can be exactly 0 - a real + inaccuracy introduced during case authoring. The case is invalid as a clean negative + control (it contains a genuine defect above its allowed severity). Excluding the + invalid control: B FPs = 0 = A FPs -> gate PASSES. On the remaining valid control + (s3-changelog-prose) all four arms were clean. Both readings reported; the + pre-registered rule text was not edited. +3. **Improvement gate: PASS (x3 strict).** B reliably catches, with A missing BOTH + repeats: s2-cic, s2-had, and s4-cs-unbalanced (an S4 missed-bug probe - the + highest-weighted evidence class). Further B advantages on unstable-A cases: + s2-acrt, s4-lpdid (A caught only 1/2). + +**Verdict: GO** (conditional only on accepting the invalid-control analysis in gate 2; +mechanical-literal reading is NO-GO on a control that demonstrably contains a real bug). + +## Secondary reads (informational, non-gating) + +- **Terra vs 5.5:** 3/10 vs 2/10 (+2 unstable) - "competitive with 5.5" roughly holds; + no reason to prefer it for review quality. +- **Sol@max vs Sol@xhigh:** 7/10 vs 7/10; max caught s2-nan-dof where xhigh was + unstable, but missed s4-lpdid (partial) which xhigh caught 2/2. No decisive gain at + ~1.5-2x latency (campaign p95 ~1060s vs ~590s). Default ship stays Sol@xhigh. +- **Latency (unblinded artifacts):** A ~220-420s, B ~480-900s, C ~115-445s, + D ~335-1060s. Sol@xhigh is ~1.5-2x slower than 5.5 per review. +- **Repeat stability:** B unstable on 1/8 catches; A unstable on 2/4 - B is more stable. +- **Shared blind spot:** both s4-wcr guard-order bugs missed by ALL arms (keep as + future-corpus anchors). On that case B and D instead asserted an unverified + "CI-inversion P0" with claimed R reproductions. +- **Hallucination notes:** A: 0; B: 1 (asserted R `did` execution, s4-cs r0); C: 1 + (fabricated hand-calc fixture numbers); D: 1 (claimed fwildclusterboot run). + Sol arms make confident external-tool claims - same class as known codex behavior; + grading discounted them (claims checked against ground truth). + +## Grader notes (blinded, consensus) + +M3 (=B) "most sensitive and most consistent, precise locations, quantified +reproductions"; M4 (=A) "least sensitive, repeat-unstable, cleanest on negative +controls"; M2 (=C) "uneven, drifts to adjacent blockers"; M1 (=D) "nearly as strong as +M3, verbose". + +## Provenance (committed audit record) + +This file is the sanitized, committed record of the campaign verdict (raw model +outputs stay local under the gitignored `runs/`). Local artifact integrity hashes +(sha256, first 16 hex) at verdict time: + +- `runs/gpt56/comparison.md`: `7e036c4be42d21f7` +- `runs/gpt56/comparison.blinded.md`: `6295ad0c91887d11` +- `runs/gpt56/blinding.json`: `9b075197bd4f32ab` +- Manifest: 66 run_ids, configs A,B,C,D, k=2, k_per {C:1, D:1} + +Post-verdict actions recorded here for completeness: the user accepted the GO +(including the invalid-control analysis of gate 2) on 2026-07-18; the swap shipped +with the control-arm flip in `config/configs.json`; the s3-wcr-pfloor fixture +comment overclaim that produced the contested "FP" was corrected in the same PR +(see the case.json notes), so the control is valid for future campaigns. diff --git a/tools/reviewer-eval/config/configs.json b/tools/reviewer-eval/config/configs.json index 5c6765083..196ee576e 100644 --- a/tools/reviewer-eval/config/configs.json +++ b/tools/reviewer-eval/config/configs.json @@ -1,25 +1,25 @@ { - "_comment": "Reviewer arms for the gpt-5.5 -> gpt-5.6 evaluation. treatment_fields declares which Config fields are the experimental treatment; every other confound (sandbox / action_version / cli_version, plus effort when not declared) is pinned and asserted identical across arms, and every selected multi-arm subset must decompose into single-field contrasts (see engine.runner). cli_version pinned to codex-cli 0.144.5, verified live 2026-07-18 to accept gpt-5.5 @ xhigh (control), gpt-5.6-sol @ xhigh AND @ max, and gpt-5.6-terra @ xhigh (invalid efforts are rejected with a 400 enum error, so accepted == executed) - one CLI serves all arms and the runner asserts the live `codex --version` matches.", + "_comment": "Reviewer arms. The 2026-07 gpt-5.5 -> gpt-5.6 evaluation ran with A as control; after the GO verdict (66-run blinded 4-arm campaign, campaigns/2026-07-gpt56-VERDICT.md) gpt-5.6-sol @ xhigh became production, so B now carries role=control and A is retained as the previous-production arm for future back-comparisons. treatment_fields declares which Config fields are the experimental treatment; every other confound (sandbox / action_version / cli_version, plus effort when not declared) is pinned and asserted identical across arms, and every selected multi-arm subset must decompose into single-field contrasts (see engine.runner). cli_version pinned to codex-cli 0.144.5, verified live 2026-07-18 to accept gpt-5.5 @ xhigh, gpt-5.6-sol @ xhigh AND @ max, and gpt-5.6-terra @ xhigh (invalid efforts are rejected with a 400 enum error, so accepted == executed) - one CLI serves all arms and the runner asserts the live `codex --version` matches.", "treatment_fields": ["model", "effort"], "arms": [ { "id": "A", - "role": "control", "model": "gpt-5.5", "effort": "xhigh", "sandbox": "read-only", "action_version": "v1", "cli_version": "codex-cli 0.144.5", - "label": "control / current production" + "label": "previous production (pre-2026-07 swap)" }, { "id": "B", + "role": "control", "model": "gpt-5.6-sol", "effort": "xhigh", "sandbox": "read-only", "action_version": "v1", "cli_version": "codex-cli 0.144.5", - "label": "primary candidate / proposed production default" + "label": "control / current production" }, { "id": "C", diff --git a/tools/reviewer-eval/corpus/cases/s3_negative/s3-wcr-pfloor-comment/case.json b/tools/reviewer-eval/corpus/cases/s3_negative/s3-wcr-pfloor-comment/case.json index 740b60049..92bb69677 100644 --- a/tools/reviewer-eval/corpus/cases/s3_negative/s3-wcr-pfloor-comment/case.json +++ b/tools/reviewer-eval/corpus/cases/s3_negative/s3-wcr-pfloor-comment/case.json @@ -24,5 +24,5 @@ "would_be_severity_if_flagged": "P3" } ], - "notes": "Negative control probing severity calibration on documented deviations: the diff is a comment-only reword inside wild_bootstrap_se next to a REGISTRY-documented deviation that is tempting to flag as an inference bug. Only the single file listed may be touched; correct behavior is no finding above P3." + "notes": "Negative control probing severity calibration on documented deviations: the diff is a comment-only reword inside wild_bootstrap_se next to a REGISTRY-documented deviation that is tempting to flag as an inference bug. Only the single file listed may be touched; correct behavior is no finding above P3. Fixture history: the original 2026-07-18 reword accidentally overclaimed ('an exact zero is never reported', contradicted by the floor >= alpha branch returning raw_p) - two reviewer arms in the first campaign correctly flagged it, invalidating the control for that run; the wording was corrected to purpose-phrasing ('to avoid reporting an exact zero') so the case is a genuinely clean control." } diff --git a/tools/reviewer-eval/corpus/cases/s3_negative/s3-wcr-pfloor-comment/inject.diff b/tools/reviewer-eval/corpus/cases/s3_negative/s3-wcr-pfloor-comment/inject.diff index 018d5ec4a..a510454ad 100644 --- a/tools/reviewer-eval/corpus/cases/s3_negative/s3-wcr-pfloor-comment/inject.diff +++ b/tools/reviewer-eval/corpus/cases/s3_negative/s3-wcr-pfloor-comment/inject.diff @@ -1,5 +1,5 @@ diff --git a/diff_diff/utils.py b/diff_diff/utils.py -index a250873f..f3901b6e 100644 +index a250873f..45514929 100644 --- a/diff_diff/utils.py +++ b/diff_diff/utils.py @@ -1019,14 +1019,15 @@ def wild_bootstrap_se( @@ -14,8 +14,8 @@ index a250873f..f3901b6e 100644 - # p-vs-CI contradiction this estimator fixes. When the floor would cross - # alpha we report the raw p-value (which is < alpha in exactly those cases), - # so the significance verdict always agrees with the inverted CI. -+ # Floor the reported p-value at the bootstrap resolution 1/(n_valid+1) so an -+ # exact zero is never reported (a documented departure from boottest, which ++ # Floor the reported p-value at the bootstrap resolution 1/(n_valid+1) to ++ # avoid reporting an exact zero (a documented departure from boottest, which + # can return p == 0) — but NEVER let the floor reach the significance level. + # With very few valid draws the resolution floor can exceed alpha, and + # flooring there would flip a bootstrap-significant result (0 outside the diff --git a/tools/reviewer-eval/run_eval.py b/tools/reviewer-eval/run_eval.py index 203a8bd4d..3bab5eacc 100644 --- a/tools/reviewer-eval/run_eval.py +++ b/tools/reviewer-eval/run_eval.py @@ -17,7 +17,7 @@ Usage: python tools/reviewer-eval/run_eval.py verify-corpus - python tools/reviewer-eval/run_eval.py smoke --configs A + python tools/reviewer-eval/run_eval.py smoke # smokes the role=control arm python tools/reviewer-eval/run_eval.py run --configs A,B,C,D --k 2 --k-per C=1,D=1 python tools/reviewer-eval/run_eval.py compare --subdir full --blinded """ @@ -151,6 +151,24 @@ def _make_configs(which: list) -> list: return [by_id[i] for i in which if i in by_id] +def _control_id() -> str: + """The id of the sole ``role='control'`` arm (current production config). + + ``smoke`` defaults to this arm, DERIVED at run time so a control flip in + configs.json can never leave the bare smoke command exercising a stale arm. + Fail-closed like ``_make_configs`` (exactly one control required). + """ + raw = _load_configs() + arms = raw.get("arms") or [] + controls = [c.get("id") for c in arms if isinstance(c, dict) and c.get("role") == "control"] + if len(controls) != 1 or not controls[0]: + raise ValueError( + f"configs.json must declare exactly one arm with role='control' " + f"(found {len(controls)})" + ) + return controls[0] + + def _treatment_fields() -> tuple: """The declared treatment fields from configs.json (default: model only). @@ -264,9 +282,12 @@ def cmd_smoke(args: argparse.Namespace) -> int: file=sys.stderr, ) return 1 - configs = _resolve_configs(args.configs) + # Bare `smoke` exercises the CURRENT production arm (the sole role=control + # entry), derived at run time so a control flip can't drift the default. + configs_arg = args.configs if args.configs is not None else _control_id() + configs = _resolve_configs(configs_arg) if configs is None: - print(_bad_configs_msg(args.configs), file=sys.stderr) + print(_bad_configs_msg(configs_arg), file=sys.stderr) return 1 if _verify_cases(loader, cases): return 1 @@ -571,7 +592,11 @@ def main() -> int: pv.set_defaults(func=cmd_verify_corpus) ps = sub.add_parser("smoke", help="tiny end-to-end run (1 case; first codex call)") - ps.add_argument("--configs", default="A") + ps.add_argument( + "--configs", + default=None, + help="arm ids to smoke (default: the role=control arm from configs.json)", + ) ps.add_argument("--strata", nargs="*", default=None) ps.add_argument("--k", type=int, default=1) ps.add_argument(