-
Notifications
You must be signed in to change notification settings - Fork 1
maint-78: derive pilot candidates from the registry + auto-trigger (#2819 move 1) #2831
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,49 @@ | ||
| { | ||
| "candidates": [ | ||
| {"provider":"openai","model_id":"gpt-5.4","role":"incumbent"}, | ||
| {"provider":"openai","model_id":"gpt-5.6-terra","role":"candidate"}, | ||
| {"provider":"openai","model_id":"gpt-5.6-sol","role":"candidate"}, | ||
| {"provider":"anthropic","model_id":"claude-opus-4-6","role":"incumbent"}, | ||
| {"provider":"anthropic","model_id":"claude-opus-4-8","role":"candidate"}, | ||
| {"provider":"anthropic","model_id":"claude-sonnet-5","role":"candidate"}, | ||
| {"provider":"github-models","model_id":"codex-mini-latest","role":"incumbent"} | ||
| { | ||
| "provider": "anthropic", | ||
| "model_id": "claude-opus-4-6", | ||
| "role": "incumbent" | ||
| }, | ||
| { | ||
| "provider": "anthropic", | ||
| "model_id": "claude-fable-5", | ||
| "role": "candidate" | ||
| }, | ||
| { | ||
| "provider": "anthropic", | ||
| "model_id": "claude-opus-4-8", | ||
| "role": "candidate" | ||
| }, | ||
| { | ||
| "provider": "anthropic", | ||
| "model_id": "claude-sonnet-5", | ||
| "role": "candidate" | ||
| }, | ||
| { | ||
| "provider": "github-models", | ||
| "model_id": "codex-mini-latest", | ||
| "role": "incumbent" | ||
| }, | ||
| { | ||
| "provider": "github-models", | ||
| "model_id": "openai/gpt-5", | ||
| "role": "candidate" | ||
| }, | ||
| { | ||
| "provider": "openai", | ||
| "model_id": "gpt-5.4", | ||
| "role": "incumbent" | ||
| }, | ||
| { | ||
| "provider": "openai", | ||
| "model_id": "gpt-5.6-sol", | ||
| "role": "candidate" | ||
| }, | ||
| { | ||
| "provider": "openai", | ||
| "model_id": "gpt-5.6-terra", | ||
| "role": "candidate" | ||
| } | ||
| ] | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| """Tests for tools/refresh_model_eval_candidates.py (registry-derived pilot candidates).""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import json | ||
|
|
||
| from tools import refresh_model_eval_candidates as rc | ||
|
|
||
|
|
||
| def _registry(): | ||
| return { | ||
| "selections": [ | ||
| {"provider": "openai", "profile": "verifier-balanced", "model_id": "gpt-5.4"}, | ||
| { | ||
| "provider": "anthropic", | ||
| "profile": "verifier-balanced", | ||
| "model_id": "claude-opus-4-6", | ||
| }, | ||
| {"provider": "openai", "profile": "some-other-profile", "model_id": "gpt-x"}, | ||
| ], | ||
| "models": [ | ||
| { | ||
| "provider": "openai", | ||
| "model_id": "gpt-5.4", | ||
| "lifecycle": "current", | ||
| "positioning": "incumbent-verifier", | ||
| }, | ||
| { | ||
| "provider": "openai", | ||
| "model_id": "gpt-5.6-terra", | ||
| "lifecycle": "current", | ||
| "positioning": "balanced", | ||
| }, | ||
| { | ||
| "provider": "openai", | ||
| "model_id": "gpt-5.6-luna", | ||
| "lifecycle": "current", | ||
| "positioning": "efficient", | ||
| }, # excluded | ||
| { | ||
| "provider": "openai", | ||
| "model_id": "gpt-5.5", | ||
| "lifecycle": "compatibility", | ||
| "positioning": "frontier", | ||
| }, # excluded (not current) | ||
| { | ||
| "provider": "openai", | ||
| "model_id": "gpt-blocked", | ||
| "lifecycle": "current", | ||
| "positioning": "frontier", | ||
| "blocked": True, | ||
| }, # excluded (blocked) | ||
| { | ||
| "provider": "anthropic", | ||
| "model_id": "claude-opus-4-6", | ||
| "lifecycle": "current", | ||
| "positioning": "incumbent-verifier", | ||
| }, | ||
| { | ||
| "provider": "anthropic", | ||
| "model_id": "claude-opus-4-8", | ||
| "lifecycle": "current", | ||
| "positioning": "high-capability", | ||
| }, | ||
| ], | ||
|
Comment on lines
+21
to
+65
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Cover the The fixture and assertions only exercise Also applies to: 78-82 🤖 Prompt for AI AgentsSource: Path instructions |
||
| } | ||
|
|
||
|
|
||
| def test_derive_picks_incumbent_and_verifier_candidates(): | ||
| out = rc.derive_candidates(_registry())["candidates"] | ||
| keys = [(c["provider"], c["model_id"], c["role"]) for c in out] | ||
| assert ("openai", "gpt-5.4", "incumbent") in keys | ||
| assert ("openai", "gpt-5.6-terra", "candidate") in keys | ||
| assert ("anthropic", "claude-opus-4-6", "incumbent") in keys | ||
| assert ("anthropic", "claude-opus-4-8", "candidate") in keys | ||
|
|
||
|
|
||
| def test_derive_excludes_efficient_noncurrent_and_blocked(): | ||
| models = {c["model_id"] for c in rc.derive_candidates(_registry())["candidates"]} | ||
| assert "gpt-5.6-luna" not in models # efficient | ||
| assert "gpt-5.5" not in models # not current | ||
| assert "gpt-blocked" not in models # blocked | ||
|
|
||
|
|
||
| def test_derive_only_uses_the_target_profile(): | ||
| # the some-other-profile openai selection must not become an incumbent | ||
| incumbents = { | ||
| c["model_id"] | ||
| for c in rc.derive_candidates(_registry())["candidates"] | ||
| if c["role"] == "incumbent" | ||
| } | ||
| assert "gpt-x" not in incumbents | ||
| assert incumbents == {"gpt-5.4", "claude-opus-4-6"} | ||
|
|
||
|
|
||
| def test_derive_is_deterministic_and_sorted(): | ||
| a = rc.derive_candidates(_registry()) | ||
| b = rc.derive_candidates(_registry()) | ||
| assert a == b | ||
| # candidates within a provider are sorted by model_id | ||
| anth = [c["model_id"] for c in a["candidates"] if c["provider"] == "anthropic"] | ||
| assert anth == sorted( | ||
| anth, key=lambda m: (m != "claude-opus-4-6", m) | ||
| ) # incumbent first, then sorted | ||
|
|
||
|
|
||
| def test_check_detects_drift(tmp_path, capsys): | ||
| reg = tmp_path / "reg.json" | ||
| cand = tmp_path / "cand.json" | ||
| reg.write_text(json.dumps(_registry())) | ||
| cand.write_text( | ||
| json.dumps( | ||
| {"candidates": [{"provider": "openai", "model_id": "stale", "role": "incumbent"}]} | ||
| ) | ||
| ) | ||
| rc_code = rc.main(["--registry", str(reg), "--candidates", str(cand), "--check"]) | ||
| assert rc_code == 1 # drifted | ||
|
|
||
|
|
||
| def test_write_then_check_roundtrips(tmp_path): | ||
| reg = tmp_path / "reg.json" | ||
| cand = tmp_path / "cand.json" | ||
| reg.write_text(json.dumps(_registry())) | ||
| assert rc.main(["--registry", str(reg), "--candidates", str(cand), "--write"]) == 0 | ||
| assert rc.main(["--registry", str(reg), "--candidates", str(cand), "--check"]) == 0 | ||
|
|
||
|
|
||
| def test_committed_candidates_match_registry_derivation(): | ||
| """Drift gate: the shipped config/model_eval_candidates.json must equal the derivation.""" | ||
| registry = json.loads(rc.DEFAULT_REGISTRY_PATH.read_text(encoding="utf-8")) | ||
| committed = json.loads(rc.DEFAULT_CANDIDATES_PATH.read_text(encoding="utf-8")) | ||
| assert committed == rc.derive_candidates(registry), ( | ||
| "config/model_eval_candidates.json is out of sync with config/model_registry.json; " | ||
| "run `python -m tools.refresh_model_eval_candidates --write`" | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| #!/usr/bin/env python3 | ||
| """Derive the verifier-pilot candidate set from the model registry. | ||
|
|
||
| Part of the self-feeding verifier-model promotion system (stranske/Workflows#2819), | ||
| move 1: candidates should never be hand-maintained (they drifted — a defunct | ||
| ``claude-sonnet-4-6`` was listed while the current ``claude-opus-4-8`` was omitted). | ||
| Instead derive them from ``config/model_registry.json`` so a catalog change | ||
| automatically produces the right pilot candidates. | ||
|
|
||
| For each provider selected for the target profile: | ||
| - incumbent = that profile's reviewed selection for the provider | ||
| - candidates = every OTHER current, non-blocked, same-provider catalogued model | ||
| whose positioning is not clearly non-verifier (``efficient``, | ||
| ``coding-worker-profile``). | ||
|
|
||
| ``--write`` regenerates ``config/model_eval_candidates.json``; ``--check`` exits 1 | ||
| if the committed file differs from the derived set (a drift gate). | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import argparse | ||
| import json | ||
| import sys | ||
| from pathlib import Path | ||
| from typing import Any | ||
|
|
||
| _REPO_ROOT = Path(__file__).resolve().parent.parent | ||
| DEFAULT_REGISTRY_PATH = _REPO_ROOT / "config" / "model_registry.json" | ||
| DEFAULT_CANDIDATES_PATH = _REPO_ROOT / "config" / "model_eval_candidates.json" | ||
| DEFAULT_PROFILE = "verifier-balanced" | ||
|
|
||
| # Positionings that are not verifier candidates (cost/speed tiers, worker profiles). | ||
| EXCLUDED_POSITIONINGS = frozenset({"efficient", "coding-worker-profile"}) | ||
|
|
||
|
|
||
| def derive_candidates( | ||
| registry: dict[str, Any], *, profile: str = DEFAULT_PROFILE | ||
| ) -> dict[str, Any]: | ||
| """Return the candidate set derived from the registry (pure function).""" | ||
| incumbents: dict[str, str] = { | ||
| str(sel.get("provider", "")): str(sel.get("model_id", "")) | ||
| for sel in registry.get("selections", []) | ||
| if sel.get("profile") == profile and sel.get("provider") and sel.get("model_id") | ||
| } | ||
| models = registry.get("models", []) | ||
|
|
||
| candidates: list[dict[str, str]] = [] | ||
| for provider in sorted(incumbents): | ||
| incumbent = incumbents[provider] | ||
| candidates.append({"provider": provider, "model_id": incumbent, "role": "incumbent"}) | ||
| alternatives = sorted( | ||
| str(m.get("model_id", "")) | ||
| for m in models | ||
| if str(m.get("provider", "")) == provider | ||
| and str(m.get("model_id", "")) != incumbent | ||
| and m.get("lifecycle") == "current" | ||
| and not m.get("blocked", False) | ||
| and str(m.get("positioning", "")) not in EXCLUDED_POSITIONINGS | ||
| ) | ||
| for model_id in alternatives: | ||
| candidates.append({"provider": provider, "model_id": model_id, "role": "candidate"}) | ||
| return {"candidates": candidates} | ||
|
|
||
|
|
||
| def _load(path: Path) -> dict[str, Any]: | ||
| return json.loads(path.read_text(encoding="utf-8")) | ||
|
Comment on lines
+66
to
+67
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win Reject structurally invalid registry JSON cleanly. A valid JSON array or malformed Also applies to: 88-94 🤖 Prompt for AI AgentsSource: Path instructions |
||
|
|
||
|
|
||
| def _serialize(candidates: dict[str, Any]) -> str: | ||
| return json.dumps(candidates, indent=2) + "\n" | ||
|
|
||
|
|
||
| def main(argv: list[str] | None = None) -> int: | ||
| parser = argparse.ArgumentParser( | ||
| description="Derive verifier-pilot candidates from the registry." | ||
| ) | ||
| parser.add_argument("--registry", type=Path, default=DEFAULT_REGISTRY_PATH) | ||
| parser.add_argument("--candidates", type=Path, default=DEFAULT_CANDIDATES_PATH) | ||
| parser.add_argument("--profile", default=DEFAULT_PROFILE) | ||
| group = parser.add_mutually_exclusive_group(required=True) | ||
| group.add_argument("--write", action="store_true", help="Regenerate the candidates file.") | ||
| group.add_argument( | ||
| "--check", action="store_true", help="Exit 1 if the committed file has drifted." | ||
| ) | ||
| args = parser.parse_args(argv) | ||
|
|
||
| try: | ||
| registry = _load(args.registry) | ||
| except (OSError, json.JSONDecodeError) as exc: | ||
| print(f"cannot read registry: {exc}", file=sys.stderr) | ||
| return 2 | ||
|
|
||
| derived = derive_candidates(registry, profile=args.profile) | ||
| if not derived["candidates"]: | ||
| print(f"no selections for profile {args.profile!r}; nothing to derive", file=sys.stderr) | ||
| return 2 | ||
|
|
||
| if args.write: | ||
| args.candidates.write_text(_serialize(derived), encoding="utf-8") | ||
| print(f"wrote {len(derived['candidates'])} candidate rows to {args.candidates}") | ||
| return 0 | ||
|
Comment on lines
+99
to
+102
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win Handle candidate-file write failures. An unwritable target or nonexistent parent raises 🤖 Prompt for AI AgentsSource: Path instructions |
||
|
|
||
| # --check | ||
| try: | ||
| committed = _load(args.candidates) | ||
| except (OSError, json.JSONDecodeError) as exc: | ||
| print(f"cannot read candidates file: {exc}", file=sys.stderr) | ||
| return 1 | ||
| if committed == derived: | ||
| print("candidates are in sync with the registry.") | ||
| return 0 | ||
| print( | ||
| "candidates have DRIFTED from the registry. Run " | ||
| "`python -m tools.refresh_model_eval_candidates --write` and commit.", | ||
| file=sys.stderr, | ||
| ) | ||
| return 1 | ||
|
|
||
|
|
||
| if __name__ == "__main__": # pragma: no cover | ||
| raise SystemExit(main()) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The workflow now runs automatically on matching pushes and a weekly schedule, but the inspected inventory still describes it as
workflow_dispatch/ “Manual evaluation” indocs/ci/WORKFLOW_SYSTEM.md:748, whiledocs/ci/WORKFLOWS.md:209also omits the new cadence and registry-derived behavior. This leaves operators relying on the documented topology unaware that the credential-backed, 30-case evaluation will execute automatically; update both contract documents with these trigger changes.AGENTS.md reference: AGENTS.md:L62-L65
Useful? React with 👍 / 👎.