review-provider-pr

3 files shipped with this skill. Inline text is the catalog copy. The upstream link is the source repository.

• .claude/skills/review-provider-pr/SKILL.md7.8 KB0705169c86c6upstream ↗

  ---
  name: review-provider-pr
  description: Review a Decision Bench pull request that adds a model provider or publishes results. Reviews the adapter code, checks the published files, re-runs the model 3 to 5 times, compares the runs row by row, checks latency against the network baseline, and drafts the GitHub review for the maintainer. Use when a PR changes an adapter in decision_bench/, config/models.json or a results/<suite>/<model-id>/ folder, or when asked to verify or benchmark a submitted model.
  ---
  
  # Review a provider or results PR
  
  The goal is to decide whether the submitted number is what the model gives today, through the named provider, and
  whether the code that produced it is safe to merge. Every claim in the review must come from a file, a run or a
  command output that you can show.
  
  ## Rules for every step
  
  - Never read, print or copy `.env`. Never link or copy the maintainer's main `.env` into a PR worktree: the PR's
    code runs there and can send any key it finds to its own endpoint. The maintainer writes a worktree `.env` with
    only the reviewed provider's key.
  - Never write an endpoint URL, a hostname or an IP address into a file, a commit, a PR or a comment. Say "the
    endpoint". `scripts/check_secrets.py` scans committed files only, not comments.
  - Never post a review, approve, request changes or merge until the maintainer says so. Show the draft first.
  - Write the pass rule (step 4) down before the first run finishes, so the result cannot move it.
  
  ## 1. Set up
  
  ```sh
  gh pr view <n> --json title,author,headRefOid,files,body,commits
  git fetch origin pull/<n>/head
  git worktree add --detach /tmp/db-pr<n> FETCH_HEAD
  ```
  
  Classify the PR with the kinds in `.github/PULL_REQUEST_TEMPLATE.md`. Note every Results and Provider field that
  is empty or vague. The Results fields are the questions you would otherwise ask in the review.
  
  ## 2. Review the code
  
  Diff with `git -C /tmp/db-pr<n> diff origin/main...HEAD`. Run this step in a subagent when one is available, and
  give it this list and CONTRIBUTING.md as the rubric:
  
  - **Shared code.** List every change outside the new adapter module and every provider it reaches. Ask for a test
    that pins the old behaviour of those providers. A payload helper that four adapters share is the usual case.
  - **Recording.** Every attempt, error, token count and timing is recorded, and the exact model and request options.
    An unknown value is `null`, never `0` or a guess.
  - **No silent substitution.** No fallback model under another model's name. If `--api-model` changes the payload,
    it changes the URL too, or the adapter rejects it.
  - **Secrets.** Keys and endpoints come only from environment variables. Nothing in `config/models.json`, the run
    record or `metadata.json` carries a host, except a public pricing `source_url` and the vendor domains in `logos`.
  - **Images.** How the adapter sends them. No image bytes or local paths in the records.
  - **Network baseline.** The provider's URL is in `runner.FIXED_ENDPOINTS` or `runner.endpoint_url()`. The provider
    coverage test in `tests/test_runner.py` fails otherwise. When two provider PRs are open, the one that merges
    second adds its entry.
  - **Pricing.** A public price page in `config/models.json`.
  - **Tests.** They cover the new payload and parsing and never call a real model.
  
  ## 3. Check the published files
  
  In `/tmp/db-pr<n>`:
  
  ```sh
  make check
  python3 -m decision_bench leaderboard && git diff --exit-code results/
  ```
  
  `validate` already rescores `predictions.jsonl` and `scores.json` against the corpus. The second command shows that
  the committed leaderboard matches. In `metadata.json`, check:
  
  - `harness.git_commit` is in the PR and `harness.git_dirty` is `false`. If it is `true`, ask what was not committed.
  - coverage is every row, and `corpus.sha256` is the current corpus;
  - the run dates and the request options (`run.jobs`, `run.max_attempts`, `run.timeout_seconds`);
  - `run.network_baseline` and `totals.server_time_p50_ms_estimate`, when the harness recorded them.
  
  ## 4. Re-run the model
  
  Ask the maintainer to write `/tmp/db-pr<n>/.env` with only the environment variable that the provider reads (the
  Provider section of the PR names it), set to a key of the same kind that a customer gets. Then:
  
  ```sh
  cd /tmp/db-pr<n>
  python3 -m decision_bench run --model <model-id> --limit 20 --run-id <model-id>-pr<n>-smoke
  python3 -m decision_bench run --model <model-id> --run-id <model-id>-pr<n>-r1 \
      --jobs <run.jobs> --max-attempts <run.max_attempts> --timeout <run.timeout_seconds>
  ```
  
  - Use the submitted request options. Run from a clean tree at the PR head.
  - Every run needs a new `--run-id`. The same id resumes the old run. If a run ends with errors, run the same command
    with `--retry-errors`.
  - Make 3 full runs, one after another. Runs in parallel share rate limits and change the latency.
  - If the PR changes `decision_bench/results.py`, `metrics.py`, `scoring.py` or `corpus.py`, stop and tell the
    maintainer. `compare_runs.py` scores our runs with the PR's own copy of that code.
  - Compare, with the model next to it on the leaderboard as `--against`:
  
    ```sh
    python3 -I <maintainer checkout>/.claude/skills/review-provider-pr/compare_runs.py results/<suite>/<model-id> \
        --runs <model-id>-pr<n>-r1,<model-id>-pr<n>-r2,<model-id>-pr<n>-r3 --against <neighbour-model-id>
    ```
  
  **Pass rule.** The submitted correct count passes when it is not above our maximum plus 2 standard deviations (the
  standard deviation has a floor of 1 row). If it fails after 3 runs, make 2 more and decide on all 5. A submitted
  count far below ours is not inflation, but report it.
  
  **Read the output.**
  
  - Run-to-run noise on this corpus is small (about 1 to 2 rows). The 95% interval on one run is about plus or minus
    15 rows, so the paired comparison against the neighbour is what tells you if a rank changes.
  - Rows where the submitted run differs from all of ours, with a confident probability on the other answer, are not
    sampling noise. If the input tokens are the same, the requests were the same and the model or the deployment
    behind the key changed.
  - If our runs share identical probabilities on many more rows with each other than with the submitted run, the
    submitted run came from a different backend.
  - The client location does not change accuracy. Do not ask for a re-run from another region to explain an accuracy
    gap.
  
  ## 5. Check latency
  
  The median latency is what the client sees: model time, provider serving time and the client's network. Each run
  records the network baseline and an estimated server time (see `docs/protocol.md`). Compare the server time with the
  submitter's, not the median. A lower submitted median usually means that their client was nearer the endpoint. Get
  the client location from the Results fields.
  
  ## 6. Draft the review
  
  Write it in the maintainer's voice, short and direct, and show it before posting:
  
  - one opening paragraph: what checks out and what does not;
  - a table of every run, submitted and ours: date, correct, accuracy, median latency, server time;
  - one line with our mean, spread and the result of the pass rule;
  - the deviations: the categories whose counts differ, and the rows that differ from all of our runs;
  - latency in one sentence;
  - questions only where the answer changes the decision;
  - requested changes as a GitHub task list (`- [ ] ...`).
  
  No diagrams, no commands for the author, no notes on how to read a table. Post only when told, with
  `gh pr review <n> --request-changes --body-file <file>` (or `--approve`, or `--comment`).
  
  ## 7. After the decision
  
  - If another provider PR is open, check which one adds the `FIXED_ENDPOINTS` entry, and re-run CI on the one that
    merges second.
  - After the merge, `python3 -m decision_bench report` and the site build pass on `main`.
  - Clean up: delete the worktree `.env`, copy any run folder you want to keep (`runs/` is ignored), then
    `git worktree remove /tmp/db-pr<n>`.
  

• .claude/skills/review-provider-pr/compare_runs.py6.1 KBe107e3fe61dcupstream ↗

  """Compare a submitted results folder with our repeat runs of the same model.
  
  Step 4 of SKILL.md gives the command. Exit code 0 when the submitted correct count is inside our range
  (max + 2 sd), 1 when it is not or when a run cannot be compared.
  """
  import argparse
  import collections
  import json
  import os
  import statistics
  import sys
  from pathlib import Path
  
  sys.path.insert(0, os.getcwd())
  from decision_bench.corpus import load_cases, load_manifest  # noqa: E402
  from decision_bench.metrics import paired_comparison  # noqa: E402
  from decision_bench.results import _fmt, export_run, records_from_predictions  # noqa: E402
  from decision_bench.runner import read_jsonl  # noqa: E402
  
  MIN_SD = 1.0
  
  
  def load_folder(folder):
      folder = Path(folder)
      metadata = json.loads((folder / "metadata.json").read_text())
      return metadata, read_jsonl(folder / "predictions.jsonl")
  
  
  def pass_rule(ours, submitted):
      """(mean, sd, limit, passed): the submitted count passes when it is not above our max + 2 sd, sd at least 1 row."""
      sd = max(statistics.stdev(ours) if len(ours) > 1 else 0.0, MIN_SD)
      limit = max(ours) + 2 * sd
      return statistics.mean(ours), sd, limit, submitted <= limit
  
  
  def by_row(predictions):
      return {p["row_id"]: p for p in predictions}
  
  
  def gold_probability(p):
      return (p.get("probabilities") or {}).get(p["gold"], 0)
  
  
  def main():
      parser = argparse.ArgumentParser()
      parser.add_argument("submitted", help="results/<suite>/<model-id> folder from the PR")
      parser.add_argument("--runs", required=True, help="Comma-separated run ids (or published folders) of our runs")
      parser.add_argument("--against", help="A published model id for a paired comparison, for example the neighbour")
      args = parser.parse_args()
  
      cases, manifest = load_cases(), load_manifest()
      case_map = {c["id"]: c for c in cases}
      runs = {"submitted": load_folder(args.submitted)}
      for run in args.runs.split(","):
          if Path(run).is_dir():
              runs[run] = load_folder(run)
          else:
              metadata, predictions, _ = export_run(run, cases, manifest)
              runs[run] = (metadata, predictions)
      rows = {name: by_row(predictions) for name, (_, predictions) in runs.items()}
      ours = [name for name in runs if name != "submitted"]
      for name in ours:
          if rows[name].keys() != rows["submitted"].keys():
              sys.exit(f"{name} has {len(rows[name])} rows and the submitted run has {len(rows['submitted'])}. "
                       "Finish the run (the same command resumes it) before you compare.")
          if any(p["status"] != "ok" for p in rows[name].values()):
              sys.exit(f"{name} has error rows. Run the same command with --retry-errors before you compare.")
  
      print("| Run | Correct | Accuracy | Median latency | Server time | Errors | git_dirty |")
      print("| --- | --- | --- | --- | --- | --- | --- |")
      counts = {}
      for name, (metadata, predictions) in runs.items():
          totals = metadata["totals"]
          counts[name] = sum(p["correct"] for p in predictions)
          errors = sum(p["status"] != "ok" for p in predictions)
          print(f"| {name} | {counts[name]} | {counts[name] / len(predictions):.2%} | "
                f"{_fmt(totals['latency_p50_ms'], 'ms')} | {_fmt(totals.get('server_time_p50_ms_estimate'), 'ms')} | "
                f"{errors} | {metadata['harness']['git_dirty']} |")
  
      ours_counts = [counts[name] for name in ours]
      mean, sd, limit, inside = pass_rule(ours_counts, counts["submitted"])
      print(f"\nOurs: mean {mean:.1f}, sd {sd:.2f} (floor {MIN_SD}), range {min(ours_counts)}-{max(ours_counts)}.")
      print(f"Submitted {counts['submitted']} is {(counts['submitted'] - mean) / sd:+.2f} sd from our mean; "
            f"limit max + 2 sd = {limit:.1f} -> {'PASS' if inside else 'FAIL'}")
  
      print("\nCategories where the correct count is not the same in every run:")
      categories = sorted({p["category"] for p in rows["submitted"].values()})
      for category in categories:
          per_run = [sum(p["correct"] for p in rows[name].values() if p["category"] == category) for name in runs]
          if len(set(per_run)) > 1:
              print(f"  {category}: " + ", ".join(f"{name} {n}" for name, n in zip(runs, per_run)))
  
      submitted = rows["submitted"]
      split = [i for i in submitted if all(rows[name][i]["correct"] != submitted[i]["correct"] for name in ours)]
      print(f"\nRows where the submitted run differs from all {len(ours)} of ours: {len(split)} "
            f"({sum(submitted[i]['correct'] for i in split)} go the submitted run's way)")
      for i in split:
          answers = collections.Counter(rows[name][i]["answer"] for name in ours)
          gold_ours = [gold_probability(rows[name][i]) for name in ours]
          print(f"  {i}: gold {submitted[i]['gold']}, submitted {submitted[i]['answer']} "
                f"(p(gold) {gold_probability(submitted[i]):.2f}), ours {dict(answers)} "
                f"(p(gold) {min(gold_ours):.2f}-{max(gold_ours):.2f})")
  
      def same_probabilities(a, b):
          return sum(rows[a][i].get("probabilities") is not None
                     and rows[a][i].get("probabilities") == rows[b][i].get("probabilities") for i in submitted)
  
      pairs = [same_probabilities(a, b) for n, a in enumerate(ours) for b in ours[n + 1:]]
      print("\nRows with identical probabilities: "
            + (f"between our runs {min(pairs)}-{max(pairs)}; " if pairs else "")
            + "submitted vs each of ours " + ", ".join(str(same_probabilities("submitted", name)) for name in ours))
  
      if args.against:
          _, reference = load_folder(Path(args.submitted).parent / args.against)
          b = records_from_predictions(reference, args.against)[0]
          print(f"\nPaired accuracy difference against {args.against} (task-cluster bootstrap 95% interval):")
          for name, (_, predictions) in runs.items():
              c = paired_comparison(records_from_predictions(predictions, name)[0], b, case_map)
              low, high = c["cluster_bootstrap_ci95"]
              print(f"  {name}: {c['accuracy_difference']:+.4f} [{low:+.4f}, {high:+.4f}]")
      return 0 if inside else 1
  
  
  if __name__ == "__main__":
      sys.exit(main())
  

• README.mdupstream ↗

  Contents are not inlined in the catalog. Open the upstream file.

Plain-text copy, no login: /skill-bundles/atlanai-decision-bench-claude-skills-review-provider-pr.md