From 72969beb17a8fe401711f0a2e66d9bb92abc5d09 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 11:03:29 +0000 Subject: [PATCH] ci: fail PRs that edit or delete a published model run model_run_key is the immutable benchmark identity of one model plus one option set; benchmark files and the warehouse's dim_model_run key on it. Add a PR check that dumps (model_run_key, model_key, options) for every run on the base branch and on the PR head and fails when a base run is missing or has a different model_key or options. Adding runs stays allowed; slug, release dates, Models.dev metadata and `active` are not compared. scripts/check_model_run_immutability.py has two subcommands: `dump` (NDJSON of the immutable fields, sorted, with a fingerprint) and `check` (compare two dumps, print a report, exit 1 on violation). The workflow installs the base and PR versions of the package into separate venvs and runs the PR's copy of the script by file path against each, so the check does not depend on the base branch already carrying the script. Document the freeze rule in AGENTS.md. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01XSAqY76fnCZj5zFnM7gHFL Co-authored-by: nflechner <60731786+nflechner@users.noreply.github.com> --- .github/workflows/model-run-immutability.yml | 70 +++++++ AGENTS.md | 10 + scripts/check_model_run_immutability.py | 188 ++++++++++++++++++ .../unit/test_check_model_run_immutability.py | 154 ++++++++++++++ 4 files changed, 422 insertions(+) create mode 100644 .github/workflows/model-run-immutability.yml create mode 100644 scripts/check_model_run_immutability.py create mode 100644 tests/unit/test_check_model_run_immutability.py diff --git a/.github/workflows/model-run-immutability.yml b/.github/workflows/model-run-immutability.yml new file mode 100644 index 0000000..1d12e62 --- /dev/null +++ b/.github/workflows/model-run-immutability.yml @@ -0,0 +1,70 @@ +# Guard: published LLM model runs never change. +# +# model_run_key is the immutable benchmark identity of one model plus one option +# set; benchmark result files and the data warehouse's dim_model_run key on it. +# On every PR that touches the registry this job dumps the immutable fields +# (model_run_key, model_key, options) of every run on the BASE branch and on the +# PR head, then fails if any base run is missing from the PR or has different +# model_key/options. Adding runs is fine; editing or deleting one is not — declare +# a new variant instead. Mutable fields (slug, release dates, Models.dev metadata, +# active) are not compared. +# +# Both dumps are produced by the PR's copy of scripts/check_model_run_immutability.py, +# run BY FILE PATH against two separate installs of the package: the base branch's +# and the PR's. Running by path keeps the checkout directory off sys.path, so +# `import utils` resolves to whichever install the interpreter carries. + +name: Model run immutability + +on: + pull_request: + branches: [main] + paths: + - "utils/llm/**" + - "scripts/check_model_run_immutability.py" + - ".github/workflows/model-run-immutability.yml" + +permissions: + contents: read + +jobs: + check: + runs-on: ubuntu-latest + steps: + - name: Check out PR head + uses: actions/checkout@v7 + with: + path: pr + + - name: Check out base branch + uses: actions/checkout@v7 + with: + ref: ${{ github.base_ref }} + path: base + + - uses: actions/setup-python@v7 + with: + python-version: "3.10" + + - name: Install the base branch's package + run: | + python -m venv "$RUNNER_TEMP/venv-base" + "$RUNNER_TEMP/venv-base/bin/python" -m pip install --quiet ./base + + - name: Install the PR's package + run: | + python -m venv "$RUNNER_TEMP/venv-pr" + "$RUNNER_TEMP/venv-pr/bin/python" -m pip install --quiet ./pr + + - name: Dump model runs on base and PR + run: | + "$RUNNER_TEMP/venv-base/bin/python" pr/scripts/check_model_run_immutability.py dump \ + --output "$RUNNER_TEMP/base.ndjson" + "$RUNNER_TEMP/venv-pr/bin/python" pr/scripts/check_model_run_immutability.py dump \ + --output "$RUNNER_TEMP/pr.ndjson" + echo "base: $(wc -l < "$RUNNER_TEMP/base.ndjson") runs, pr: $(wc -l < "$RUNNER_TEMP/pr.ndjson") runs" + + - name: Verify every published run is unchanged + run: | + "$RUNNER_TEMP/venv-pr/bin/python" pr/scripts/check_model_run_immutability.py check \ + "$RUNNER_TEMP/base.ndjson" "$RUNNER_TEMP/pr.ndjson" diff --git a/AGENTS.md b/AGENTS.md index 7aebb97..999a65e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -104,6 +104,16 @@ PY use `model_key` plus `-run-variant-XX`; Artificial Analysis-backed runs use `model_key` plus `-aa-run-variant-XX`. In both cases, `XX` is the next two-digit variant number for that `model_key`, and the key must never change after publication. +- Once a run is on `main` it is frozen: never edit its `model_key` or `options` + and never delete it. CI enforces this on every PR + (`.github/workflows/model-run-immutability.yml` runs + `scripts/check_model_run_immutability.py` against the base branch). To change + options, add a new run with the next variant number; to retire a model, set + `active=False` on the base `Model`. + Everything else on a run or its base model is descriptive and may change + after publication: `slug` (renameable), `active`, and the release date and + Models.dev metadata, which are resolved from the checked-in snapshot and + change when it is refreshed or a `manual_release_date` correction lands. - Add every new `model_run_key` to `HISTORICAL_MODEL_RUN_KEYS` in `tests/unit/test_llm_model_runs.py`; the test ledger is the explicit list of all model-run keys ever created. - Write `slug` explicitly as the descriptive human-readable identifier. Slugs must be unique, but may be changed before or after publication when naming conventions improve. - Use `filename_safe_name` when a model-run key needs to become part of a filename; do not hand-roll lossy replacements for characters such as `/` or `*`. diff --git a/scripts/check_model_run_immutability.py b/scripts/check_model_run_immutability.py new file mode 100644 index 0000000..dba7465 --- /dev/null +++ b/scripts/check_model_run_immutability.py @@ -0,0 +1,188 @@ +"""Verify that published LLM model runs never change. + +`model_run_key` is the immutable benchmark identity of one exact model plus +option set. Downstream systems (benchmark result files, the data warehouse's +`dim_model_run`) key on it, so once a run has been merged to `main` its +identity must never be edited or deleted: a new option set is a NEW run with +the next variant number, never an edit of an existing one. + +This script enforces that in CI by comparing two dumps of the registry, one +from the pull request's base branch and one from the PR head: + + python scripts/check_model_run_immutability.py dump > base.ndjson # on base + python scripts/check_model_run_immutability.py dump > pr.ndjson # on PR + python scripts/check_model_run_immutability.py check base.ndjson pr.ndjson + +`check` fails when a run present on the base is missing from the PR, or is +present with a different `model_key` or `options`. Adding runs is always +allowed. Everything else is deliberately NOT compared: `slug` and `active` +are documented as changeable (AGENTS.md), and release dates plus the +`models_dev_*` metadata are resolved from a refreshable snapshot, so none of +them is a stable part of a run's identity. + +`dump` imports `utils.llm.model_runs` from whichever interpreter runs it and +uses only the long-stable public attributes (`MODEL_RUNS`, `model_run_key`, +`model_key`, `options`), so the PR's copy of this script can dump the BASE +checkout too: run it by file path with a Python whose site-packages holds the +base version of the package (see .github/workflows/model-run-immutability.yml). +Running by path keeps the current directory off `sys.path`, so `utils` resolves +to the installed package, not to a source tree that happens to be the cwd. +""" + +from __future__ import annotations + +import argparse +import hashlib +import json +import sys +from dataclasses import dataclass +from pathlib import Path +from typing import Any, Iterable + +# The identity of a run. Everything else on a ModelRun/Model may change. +IMMUTABLE_FIELDS: tuple[str, ...] = ("model_run_key", "model_key", "options") + + +def canonical_json(value: Any) -> str: + """Serialize `value` deterministically so equal options compare equal as text.""" + return json.dumps(value, sort_keys=True, separators=(",", ":")) + + +def fingerprint(record: dict[str, Any]) -> str: + """Return a SHA256 over the immutable fields of one dumped record.""" + payload = canonical_json({field: record.get(field) for field in IMMUTABLE_FIELDS}) + return hashlib.sha256(payload.encode("utf-8")).hexdigest() + + +def registry_records() -> list[dict[str, Any]]: + """Return one record per registered model run, sorted by `model_run_key`.""" + from utils.llm import model_runs # imported lazily: `check` needs no registry + + records = [ + { + "model_run_key": run.model_run_key, + "model_key": run.model_key, + "options": run.options, + } + for run in model_runs.MODEL_RUNS + ] + for record in records: + record["fingerprint"] = fingerprint(record) + return sorted(records, key=lambda record: record["model_run_key"]) + + +def dump_ndjson(records: Iterable[dict[str, Any]]) -> str: + """Render records as newline-delimited canonical JSON, one run per line.""" + return "".join(f"{canonical_json(record)}\n" for record in records) + + +def load_ndjson(path: Path) -> dict[str, dict[str, Any]]: + """Load a dump into a mapping keyed by `model_run_key`. + + Rejects an empty dump: an import failure upstream must not masquerade as + "no runs on the base branch, nothing to protect". + """ + records: dict[str, dict[str, Any]] = {} + for line_number, line in enumerate(path.read_text().splitlines(), start=1): + if not line.strip(): + continue + record = json.loads(line) + key = record["model_run_key"] + if key in records: + raise ValueError(f"{path}:{line_number}: duplicate model_run_key {key}") + records[key] = record + if not records: + raise ValueError(f"{path}: dump contains no model runs") + return records + + +@dataclass(frozen=True) +class Violation: + """One published run that the candidate registry removed or altered.""" + + model_run_key: str + kind: str # "removed" | "changed" + detail: str + + def __str__(self) -> str: + """Render as one report line.""" + return f"[{self.kind}] {self.model_run_key}: {self.detail}" + + +def find_violations( + base: dict[str, dict[str, Any]], + candidate: dict[str, dict[str, Any]], +) -> list[Violation]: + """Return every base run that is missing from, or altered in, the candidate. + + Runs only present in the candidate are additions and never a violation. + """ + violations: list[Violation] = [] + for key in sorted(base): + if key not in candidate: + violations.append(Violation(key, "removed", "present on base, missing from PR")) + continue + for field in IMMUTABLE_FIELDS: + before = canonical_json(base[key].get(field)) + after = canonical_json(candidate[key].get(field)) + if before != after: + violations.append( + Violation(key, "changed", f"{field} was {before} and is now {after}") + ) + return violations + + +def check(base_path: Path, candidate_path: Path) -> int: + """Compare two dumps, print a report, and return the process exit code.""" + base = load_ndjson(base_path) + candidate = load_ndjson(candidate_path) + violations = find_violations(base, candidate) + added = sorted(set(candidate) - set(base)) + + print(f"Base runs verified: {len(base)}") + print(f"Runs added in PR: {len(added)}") + for key in added: + print(f" + {key}") + + if not violations: + print("OK: every published model run is unchanged.") + return 0 + + print("") + print(f"FAIL: {len(violations)} published model run(s) were removed or changed.") + print("model_run_key is immutable once merged: declare a new run with the next") + print("variant number instead of editing or deleting an existing one.") + for violation in violations: + print(f" {violation}") + return 1 + + +def main(argv: list[str] | None = None) -> int: + """Parse arguments and dispatch to `dump` or `check`.""" + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + subparsers = parser.add_subparsers(dest="command", required=True) + + dump_parser = subparsers.add_parser( + "dump", help="write the registry's immutable fields as NDJSON" + ) + dump_parser.add_argument( + "--output", type=Path, default=None, help="write to this path instead of stdout" + ) + + check_parser = subparsers.add_parser("check", help="compare a base dump against a PR dump") + check_parser.add_argument("base", type=Path, help="dump taken on the PR's base branch") + check_parser.add_argument("candidate", type=Path, help="dump taken on the PR head") + + args = parser.parse_args(argv) + if args.command == "dump": + text = dump_ndjson(registry_records()) + if args.output is None: + sys.stdout.write(text) + else: + args.output.write_text(text) + return 0 + return check(args.base, args.candidate) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/unit/test_check_model_run_immutability.py b/tests/unit/test_check_model_run_immutability.py new file mode 100644 index 0000000..739ab54 --- /dev/null +++ b/tests/unit/test_check_model_run_immutability.py @@ -0,0 +1,154 @@ +"""Tests for the CI model-run immutability check.""" + +import json +from pathlib import Path + +from scripts import check_model_run_immutability as immutability +from utils.llm import model_runs + + +def _dump(records: list[dict]) -> dict[str, dict]: + return {record["model_run_key"]: record for record in records} + + +BASE = [ + { + "model_run_key": "a-run-variant-01", + "model_key": "a", + "options": {"max_tokens": 1024, "temperature": 0}, + }, + {"model_run_key": "b-run-variant-01", "model_key": "b", "options": {}}, +] + + +# --- find_violations ----------------------------------------------------------- + + +def test_identical_registries_have_no_violations() -> None: + """A PR that touches nothing passes.""" + assert immutability.find_violations(_dump(BASE), _dump(BASE)) == [] + + +def test_adding_a_run_is_allowed() -> None: + """New variants are the sanctioned way to change options.""" + candidate = BASE + [ + {"model_run_key": "a-run-variant-02", "model_key": "a", "options": {"max_tokens": 2048}} + ] + assert immutability.find_violations(_dump(BASE), _dump(candidate)) == [] + + +def test_removing_a_published_run_is_a_violation() -> None: + """Deleting a run breaks every downstream reference to its key.""" + violations = immutability.find_violations(_dump(BASE), _dump(BASE[:1])) + + assert [(v.model_run_key, v.kind) for v in violations] == [("b-run-variant-01", "removed")] + + +def test_changing_options_is_a_violation_and_names_the_field() -> None: + """Editing options in place is exactly what the check exists to catch.""" + candidate = [dict(BASE[0], options={"max_tokens": 1024, "temperature": 0.5}), BASE[1]] + + violations = immutability.find_violations(_dump(BASE), _dump(candidate)) + + assert len(violations) == 1 + assert violations[0].kind == "changed" + assert violations[0].model_run_key == "a-run-variant-01" + assert "options" in violations[0].detail + assert '"temperature":0.5' in violations[0].detail + + +def test_changing_model_key_is_a_violation() -> None: + """Re-pointing a run at another base model changes its identity.""" + candidate = [dict(BASE[0], model_key="a-v2"), BASE[1]] + + violations = immutability.find_violations(_dump(BASE), _dump(candidate)) + + assert [(v.model_run_key, v.kind) for v in violations] == [("a-run-variant-01", "changed")] + assert "model_key" in violations[0].detail + + +def test_option_key_order_does_not_matter() -> None: + """Only the option VALUES are identity; dict ordering is a serialization detail.""" + reordered = [dict(BASE[0], options={"temperature": 0, "max_tokens": 1024}), BASE[1]] + + assert immutability.find_violations(_dump(BASE), _dump(reordered)) == [] + + +# --- dump / load --------------------------------------------------------------- + + +def test_registry_dump_covers_every_run_sorted_with_only_immutable_fields() -> None: + """The dump is the full registry, ordered for stable diffs, nothing mutable in it.""" + records = immutability.registry_records() + + assert [r["model_run_key"] for r in records] == sorted( + run.model_run_key for run in model_runs.MODEL_RUNS + ) + assert len(records) == len(model_runs.MODEL_RUNS) + assert set(records[0]) == set(immutability.IMMUTABLE_FIELDS) | {"fingerprint"} + + +def test_dump_round_trips_through_load(tmp_path: Path) -> None: + """What `dump` writes, `load_ndjson` reads back unchanged.""" + records = immutability.registry_records() + path = tmp_path / "dump.ndjson" + path.write_text(immutability.dump_ndjson(records)) + + loaded = immutability.load_ndjson(path) + + assert loaded == {r["model_run_key"]: r for r in records} + assert all(json.loads(line) for line in path.read_text().splitlines()) + + +def test_load_rejects_empty_dump(tmp_path: Path) -> None: + """An empty base dump must fail loudly, not pass as 'nothing to protect'.""" + path = tmp_path / "empty.ndjson" + path.write_text("\n") + + try: + immutability.load_ndjson(path) + except ValueError as exc: + assert "no model runs" in str(exc) + else: + raise AssertionError("expected ValueError for an empty dump") + + +def test_fingerprint_ignores_mutable_fields_and_option_order() -> None: + """Same identity, same fingerprint, whatever else rides along on the record.""" + base = {"model_run_key": "k", "model_key": "m", "options": {"a": 1, "b": 2}} + same = {"model_run_key": "k", "model_key": "m", "options": {"b": 2, "a": 1}, "slug": "x"} + different = {"model_run_key": "k", "model_key": "m", "options": {"a": 1, "b": 3}} + + assert immutability.fingerprint(base) == immutability.fingerprint(same) + assert immutability.fingerprint(base) != immutability.fingerprint(different) + + +# --- CLI ----------------------------------------------------------------------- + + +def test_cli_check_exit_codes(tmp_path: Path, capsys) -> None: + """`check` returns 0 when unchanged and 1 with a readable report on a violation.""" + base_path = tmp_path / "base.ndjson" + base_path.write_text(immutability.dump_ndjson(BASE)) + ok_path = tmp_path / "ok.ndjson" + ok_path.write_text( + immutability.dump_ndjson(BASE + [dict(BASE[0], model_run_key="a-run-variant-02")]) + ) + bad_path = tmp_path / "bad.ndjson" + bad_path.write_text(immutability.dump_ndjson([dict(BASE[0], options={}), BASE[1]])) + + assert immutability.main(["check", str(base_path), str(ok_path)]) == 0 + assert "Runs added in PR: 1" in capsys.readouterr().out + + assert immutability.main(["check", str(base_path), str(bad_path)]) == 1 + out = capsys.readouterr().out + assert "FAIL" in out + assert "[changed] a-run-variant-01" in out + + +def test_cli_dump_writes_registry(tmp_path: Path) -> None: + """`dump --output` writes the same document `registry_records` describes.""" + out = tmp_path / "registry.ndjson" + + assert immutability.main(["dump", "--output", str(out)]) == 0 + assert out.read_text() == immutability.dump_ndjson(immutability.registry_records())