From 9a0ef8fbbb2655b2271f04f86981dc345e552970 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Tue, 27 Oct 2026 09:17:08 +0000 Subject: [PATCH] fix(eval): reuse reference settings for PR cost previews (#3738) Co-authored-by: Gergo Magyar --- .github/workflows/release-evaluation.yml | 2 +- eval/tests/test_ec2_workflows.py | 6 +++ eval/tests/test_pr_evaluation.py | 55 +++++++++++++++++++++++- eval/workflow_bench/pr_evaluation.py | 22 ++++++++-- 4 files changed, 78 insertions(+), 7 deletions(-) diff --git a/.github/workflows/release-evaluation.yml b/.github/workflows/release-evaluation.yml index 5bc4d25ed..d4376ef15 100644 --- a/.github/workflows/release-evaluation.yml +++ b/.github/workflows/release-evaluation.yml @@ -294,7 +294,7 @@ jobs: run: | uv run --locked --extra dev python -m workflow_bench.pr_evaluation baseline \ --repo "$GITHUB_REPOSITORY" --run-id "$COMPARISON_RUN_ID" \ - --harness-sha "$GITHUB_SHA" --model "$MODEL" --effort "$EFFORT" \ + --harness-sha "$GITHUB_SHA" --env-out "$GITHUB_ENV" \ --out "$RUNNER_TEMP/release-evaluation/previous.json" # Both runtimes grade the pinned task source against one dependency tree # built from that commit's lockfile, never the runtime under test's. diff --git a/eval/tests/test_ec2_workflows.py b/eval/tests/test_ec2_workflows.py index af7e6e71e..3bfe5a0ad 100644 --- a/eval/tests/test_ec2_workflows.py +++ b/eval/tests/test_ec2_workflows.py @@ -118,6 +118,12 @@ def test_pr_preview_uses_trusted_main_and_authenticates_history_before_paid_sess paid = next(step for step in steps if "workflow_bench.runner" in step.get("run", "")) assert steps.index(baseline) < steps.index(paid) assert "inputs.candidate_pr != ''" in baseline["if"] + assert '--env-out "$GITHUB_ENV"' in baseline["run"] + assert "--model" not in baseline["run"] and "--effort" not in baseline["run"] + prepare = next(step for step in steps if step.get("name") == "Prepare pinned task set") + assert steps.index(baseline) < steps.index(prepare) < steps.index(paid) + for step in (prepare, paid): + assert '--model "$MODEL" --effort "$EFFORT"' in step["run"] assert 'arms=(baseline)' in paid["run"] dependencies = next(step for step in steps if "workflow_bench.pr_evaluation dependencies" in step.get("run", "")) assert steps.index(dependencies) < steps.index(paid) diff --git a/eval/tests/test_pr_evaluation.py b/eval/tests/test_pr_evaluation.py index 0a7f4589c..5f3af3b3c 100644 --- a/eval/tests/test_pr_evaluation.py +++ b/eval/tests/test_pr_evaluation.py @@ -509,6 +509,56 @@ def test_baseline_download_authenticates_run_artifact_and_recomputes_report(gith assert pr_evaluation.fetch_baseline(REPO, 42, HARNESS, "gpt-5.6-sol", "xhigh") == github["report"] +@pytest.mark.parametrize( + "model,effort", + [ + ("gpt-5.6-sol", "low"), + ("gpt-5.5", "medium"), + ("gpt-5.6-sol\nOTHER=value", "low"), + ("gpt-5.6-sol", "low\rOTHER=value"), + ], +) +def test_baseline_cli_inherits_authenticated_settings(github, tmp_path, monkeypatch, model, effort): + previous = github["report"] + metadata = {**previous, "model": model, "effort": effort} + rows = [{**row, "model": model, "effort": effort} for row in previous["per_run"]] + github["report"] = build_report(rows, metadata, now=NOW) + report, environment = tmp_path / "previous.json", tmp_path / "github-env" + environment.write_text("EXISTING=value\n") + monkeypatch.setattr( + "sys.argv", + [ + "pr_evaluation", + "baseline", + "--repo", + REPO, + "--run-id", + "42", + "--harness-sha", + HARNESS, + "--out", + str(report), + "--env-out", + str(environment), + ], + ) + if "\n" in model or "\r" in effort: + with pytest.raises(ValueError, match="single-line"): + pr_evaluation.main() + assert environment.read_text() == "EXISTING=value\n" + assert not report.exists() + return + pr_evaluation.main() + assert json.loads(report.read_text()) == github["report"] + assert environment.read_text() == f"EXISTING=value\nMODEL={model}\nEFFORT={effort}\n" + + +@pytest.mark.parametrize("model,effort", [("gpt-5.5", "xhigh"), ("gpt-5.6-sol", "low")]) +def test_explicit_baseline_settings_still_require_an_exact_match(github, model, effort): + with pytest.raises(ValueError, match="does not match"): + pr_evaluation.fetch_baseline(REPO, 42, HARNESS, model, effort) + + @pytest.mark.parametrize( "mutation", [ @@ -526,10 +576,11 @@ def test_baseline_download_authenticates_run_artifact_and_recomputes_report(gith lambda g: g["report"]["arms"]["baseline"].update(mean_cost_usd=0), ], ) -def test_untrusted_or_tampered_historical_evidence_is_rejected_before_paid_work(github, mutation): +@pytest.mark.parametrize("settings", [(), ("gpt-5.6-sol", "xhigh")]) +def test_untrusted_or_tampered_historical_evidence_is_rejected_before_paid_work(github, mutation, settings): mutation(github) with pytest.raises(ValueError): - pr_evaluation.fetch_baseline(REPO, 42, HARNESS, "gpt-5.6-sol", "xhigh") + pr_evaluation.fetch_baseline(REPO, 42, HARNESS, *settings) def test_cli_writes_sanitized_candidate_report_and_marks_missing_measurements_incomplete( diff --git a/eval/workflow_bench/pr_evaluation.py b/eval/workflow_bench/pr_evaluation.py index 401aa482c..358ea1931 100644 --- a/eval/workflow_bench/pr_evaluation.py +++ b/eval/workflow_bench/pr_evaluation.py @@ -192,7 +192,9 @@ def require_comparable_harness(repo: str, previous_sha: str, harness_sha: str) - raise ValueError("historical workflow measurement protocol changed; measure a fresh baseline") -def fetch_baseline(repo: str, run_id: int, harness_sha: str, model: str, effort: str) -> dict[str, Any]: +def fetch_baseline( + repo: str, run_id: int, harness_sha: str, model: str | None = None, effort: str | None = None +) -> dict[str, Any]: from .release_evidence import eligible_runs from .release_report import suite_binding, validate_report @@ -229,8 +231,8 @@ def fetch_baseline(repo: str, run_id: int, harness_sha: str, model: str, effort: validate_report(report, runtime_sha=runtime_sha, task_set_digest=digest, task_pins=pins) if ( report["harness_sha"] != run["head_sha"] - or report["model"] != model - or report["effort"] != effort + or (model is not None and report["model"] != model) + or (effort is not None and report["effort"] != effort) or report["runs"] != 3 ): raise ValueError("historical harness, model, reasoning effort or repetition count does not match") @@ -444,10 +446,13 @@ def main() -> None: resolve.add_argument("--pr", type=int, required=True) resolve.add_argument("--sha", required=True) baseline = commands.add_parser("baseline") - for flag in ("repo", "harness-sha", "model", "effort"): + for flag in ("repo", "harness-sha"): baseline.add_argument(f"--{flag}", required=True) + for flag in ("model", "effort"): + baseline.add_argument(f"--{flag}", help="Require this setting; otherwise inherit the authenticated report") baseline.add_argument("--run-id", type=int, required=True) baseline.add_argument("--out", type=Path, required=True) + baseline.add_argument("--env-out", type=Path, help="Append authenticated MODEL/EFFORT to a GitHub environment file") dependencies = commands.add_parser("dependencies") dependencies.add_argument("--tasks", type=Path, required=True) dependencies.add_argument("--baseline", type=Path, required=True) @@ -468,8 +473,17 @@ def main() -> None: print(f"sha={resolve_pr(args.repo, args.pr, args.sha)}") elif args.command == "baseline": report = fetch_baseline(args.repo, args.run_id, args.harness_sha, args.model, args.effort) + settings = {"MODEL": report["model"], "EFFORT": report["effort"]} + if args.env_out and any( + not isinstance(value, str) or not value or any(char in value for char in "\r\n\0") + for value in settings.values() + ): + raise ValueError("historical model settings must be nonempty single-line strings") args.out.parent.mkdir(parents=True, exist_ok=True) args.out.write_text(json.dumps(report, indent=2, allow_nan=False) + "\n") + if args.env_out: + with args.env_out.open("a") as environment: + environment.writelines(f"{key}={value}\n" for key, value in settings.items()) elif args.command == "dependencies": import yaml