Verify Scoring Change

Verify a Scoring Change

A scoring change moves reported numbers. The review therefore has to establish three things: that the bug exists, that the fix removes it, and what else moves. The output is a verdict a maintainer can act on without repeating your work.

The seven questions to answer are the Scoring-Change Gate. The rules behind them are in BEST_PRACTICES.md: "Route sample-level failures to the right outcome", "Handle grader failures as instrument failures, not model failures", "Say what a metric does when its denominator is empty", and "Dict-valued scores and metric registration". Read those four sections first.

The receipts rule

Every claim about how inspect_ai behaves must be checked against the installed pin before it goes in the verdict. Find the source with uv run python -c "import inspect_ai, pathlib; print(pathlib.Path(inspect_ai.__file__).parent)" and cite file:line, or run a probe and paste its output. A claim without a receipt is left out.

The following behaviours changed within a few releases and have been cited wrongly from memory. Check each on the pin before relying on it:

• model_graded_qa and model_graded_fact return Score.unscored() on a grader parse miss (scorer/_model.py). Before inspect_ai#4048 they scored INCORRECT.
• The framework does not call a metric with an empty list. An all-unscored run reports nan under the metric's registry name without invoking it (_eval/task/results.py).
• A dict-registered scorer whose sample omits a key raises TypeError (_eval/task/results.py).
• Epoch reducers drop nan epochs. All-nan epochs make the sample unscored. A Score(value={}) breaks dict-key matching across epochs (_reducer/reducer.py).
• inspect_ai.scorer.aggregate() takes on_missing: Literal["error", "skip", "zero"] (scorer/_metrics/aggregate.py:23), returns nan when every sample is skipped or the input is empty (:148), and skips nan leaves regardless of on_missing. The repository's mean_of in src/inspect_evals/utils/metrics.py has the same parameter and, since #2230, the same empty behaviour.
• The default value_to_float maps NOANSWER to 0, so NOANSWER is already in the accuracy denominator (scorer/_metric.py).

The same rule applies to the eval. Read its README changelog, its eval.yaml version, and git log --oneline -- src/inspect_evals/<eval>/<scorer file> before judging. Whether a branch is reachable, whether a zero was a deliberate decision, and who produces the artefact being scored are all answered by that history rather than by the diff.

Setup

1. Create a worktree on the PR head: git fetch origin pull/<N>/head:pr-<N> && git worktree add .claude/worktrees/pr-<N> pr-<N>. Record the head SHA with git rev-parse pr-<N>.
2. Inside the worktree, check whether the eval has extras: grep -n "^<eval>" pyproject.toml under [project.optional-dependencies]. Extra names are not one-to-one with directories (bfcl and bfcl_v4 are separate extras), so read the matches. Then run uv sync --group dev, adding --extra <name> for each match. A missing extra makes the scorer fail at construction and voids every probe run afterwards. An eval under packages/ has no root extra and its own lockfile; run its probes and tests through its tox environment, uv run --group dev tox -e <eval> -- <pytest args>, or sync inside packages/<eval>/. Confirm uv run python -c "import inspect_evals; print(inspect_evals.__file__)" prints a path inside the worktree. If it does not, nothing you run is testing the PR.
3. Create agent_artefacts/<eval>_<version>/scoring_review_pr<N>/ in the main checkout, outside the worktree. The worktree directory is gitignored and is removed when the review ends. Write NOTES.md there as you go, and VERDICT.md at the end. The directory is gitignored; neither file is committed. The verdict reaches the maintainer through the batch runner or, when asked, as a PR comment.
4. Read the PR body and any linked issue, then set them aside. Treat each claim in the description as an item to check.

The seven questions

1. Does the bug reproduce on main? Reproduce the claimed failure in a run of the shipped task wiring, or in a test that exercises it. A stub of the code under review does not count as reproduction. Use SampleScore objects shaped like what the scorer emits, fed to the metric directly, or the task wiring with mockllm and a canned grader reply. For a grader path, feed a reply with no parseable verdict and a raised API error and record each score. For an extraction fix, run the scorer on the output the PR says was misparsed. If reproduction needs model API access you do not have, mark this UNVERIFIED.

2. Does it stop reproducing on the PR head? Use the same inputs. Record both outputs side by side. If the PR fixes a different bug from the one it describes, say which.

3. Do reported numbers move, and is that declared? Decide reachability first. A leading if not scores guard is unreachable from the framework, so changing it moves nothing. A denominator that empties inside the metric is reachable in a healthy run and does move numbers. A grader path on the default task configuration is live. If any live path moves a number: N in eval.yaml is bumped, and the README changelog and changelog.d/ fragment name which metrics move and in which direction. Run one case in each direction yourself; a "can only decrease" claim has been wrong before. If no live path moves a number, N is unchanged. X is bumped only when the task interface changed, whether or not numbers move. See TASK_VERSIONING.md for both axes.

Check for open PRs on the same eval. Two PRs that both bump 4-A to 5-A merge without a textual conflict and ship a duplicate version.

4. Does every exclusion sentinel mean one thing? List every path that yields nan, Score.unscored(), or an omitted key, and write down what each means. "The grader failed" and "the grader was not consulted because the agent complied" are different facts. If both produce nan in the same key, a metric that excludes nan drops the second group from its denominator with no record of having done so. Apply the coherence rule: model-attributed reasons (invalid_response_format, refusal, no_response) belong on scored samples in the denominator. Instrument reasons (grader_failed, scoring_failed) belong on exclusions.

5. Is anything swallowed or fallen into? Grep the changed files and their callees for: except Exception followed by a constant return; else 0.0 or if not <list>: return 0.0 in a function that divides; == CORRECT on a grader result with no unscored branch; metadata={"reason": (the key that does not migrate to Score.reason); a bare score.value["key"] in a list-registered metric. A zero that a test asserts on purpose is a decision and may stand. A zero that nothing asserts is a defect. A pre-existing defect of this kind is a blocker if the PR makes it reachable (for example, a new filter that can empty the list a pre-existing else 0.0 guards). Otherwise it is a follow-up.

6. Do tests pin both sides? Revert the PR's own source changes against its merge base, then run the PR's tests and count the failures. That count is the number of behaviours pinned. Use the merge base rather than main, so that unrelated newer changes to the eval are not pulled in, and reverse-apply the diff rather than checking out a directory, so that files the PR added are removed and changes to shared helpers such as src/inspect_evals/utils/ are included:

BASE=$(git merge-base HEAD origin/main)
git diff "$BASE" HEAD -- src/ | git apply -R
timeout 600 uv run pytest tests/<eval> -q          # count the failures; packages/ evals use the tox command from question 7
git checkout HEAD -- src/ && git clean -fd -- src/  # restore the PR head

Then confirm a correct zero still reports 0.0. For a grader path the three cases are: parse miss, raised API error, genuine negative verdict.

7. Are the checks green? Run uv run ruff check, uv run ruff format --check, uv run mypy <changed files>, uv run --group dev python tools/check_changelog.py, uv run --group dev python tools/generate_readmes.py --create-missing-readmes (then git status to confirm it changed nothing), and timeout 600 uv run pytest tests/<eval>. For an eval under packages/, its tests are excluded from the root suite and run through tox instead: uv run --group dev tox -e <eval>. A pytest run that collects no tests is recorded as not run. A fired timeout is recorded as a check that did not complete. Two mypy traps recur: a Metric-typed value is a union and cannot be called (narrow with cast(MetricProtocol, m)), and math.isnan(score.value) on the Value union (narrow with isinstance(score.value, float) first). Fork PRs get no CI until a maintainer approves it, so a local mypy is often the first anyone has run.

Common mistakes

Applying the instrument rule before finding out who writes the artefact. The words "sandbox", "file", and "JSON" describe a mechanism. They do not say who produced the content. If the model under test was instructed to write the file being read, a missing or unparseable file at that path is the model's output failing, which is a verdict (Score(0.0), in the denominator) rather than an instrument failure. The same applies to a judge that is the model under test grading itself. Read the eval's prompts before deciding whose failure it is. Attribution comes before the instrument rule.

Citing a framework behaviour from memory. Sound reasoning on a stale premise produces a confident wrong finding. See the receipts rule.

Missing a version collision between sibling PRs. See question 3.

Trusting the PR body's vocabulary claim. Grep for the constants in the code.

The verdict

## Verdict: MERGE-READY | NEEDS-WORK | UNVERIFIED

One sentence on what the PR does and whether it does it.

**Checks run:** one line per command with its result.

### Findings
1. **[REQUIRED|ADVISORY] <file>:<line>**: what is wrong, the fix, the receipt.

### Gate
| Q | Result | Receipt |
| --- | --- | --- |
| 1 Reproduces on main | yes/no/unverified | ... |

**Outcome-changing or mechanical:** one line.
**Sequencing:** sibling PRs, version collisions, dependencies. Omit if none.

### Learnings
- **skill:** a step in this skill that was missing, wrong, or unclear, and what you did instead.
- **gotcha:** an environment or tooling trap that cost time (a missing extra, a hang, a tool that failed).
- **framework:** an `inspect_ai` behaviour you checked on the pin that belongs in the receipts list, with its `file:line`.
- **eval:** something about this eval a future reviewer needs (who the judge is, where the fallback came from, which extra it needs).

**Reviewer:** model id, PR head SHA, git SHA of this file, inspect_ai version, date.

The Learnings section is how the skill improves. Write one bullet per item, each tagged with one of the four kinds, and write none under a kind that had nothing. Include the things that went wrong on the way to the verdict, not only the verdict. A maintainer reads the learnings from every run in a batch and folds the accepted ones into this file, so a note here reaches the next reviewer; a note that stays in NOTES.md does not.

UNVERIFIED is the verdict whenever question 1 or 2 could not be run. End VERDICT.md with the same content as a fenced JSON block (verdict, findings[{severity,file,line,summary,fix,receipt}], gate[{question,result,receipt}], outcome_changing, sequencing, learnings[{kind,note,receipt}] with kind one of skill, gotcha, framework, eval, and reviewer) so a batch runner can collect verdicts and learnings without parsing prose.

Do not post to GitHub. The maintainer decides what goes on the thread. If asked to post, end with *Posted by [Claude Code](https://claude.com/claude-code) on <maintainer>'s behalf.* and ping no one. Do not fix the PR unless asked.

If a framework behaviour you checked surprised you, or the tree corrected your first reading, it goes under Learnings. The maintainer decides what is folded into this file.