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())