mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-11-13 08:51:28 +00:00
fix(eval): reuse reference settings for PR cost previews (#3738)
Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
This commit is contained in:
parent
afea705e36
commit
9a0ef8fbbb
4 changed files with 78 additions and 7 deletions
2
.github/workflows/release-evaluation.yml
vendored
2
.github/workflows/release-evaluation.yml
vendored
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue