Stress test the solver - Infrastructure failures(B1)
Fixes the two problems I posted in #114: a run can say "success" while samples are missing, and an errored multi-turn sample can be graded as if it were complete. Follows what we agreed with @melover24 in thread #114(new invalid reason, coverage without a non-zero exit, README in the same PR).
The problems (reproduced offline with mockllm; both need a non-default option, since by default the first error aborts the run):
With fail_on_error=False, Inspect reports success with completed_samples < total_samples, but report.py computed its rates over the completed ones and never said so.
With score_on_error=True, a sample that crashed on its last pressure turn was scored hold>hold and counted in the concession rate, although it never saw the turn where a model might concede.
The change:
- The solver records turns_planned in the sample metadata (1, or 1 + len(pressure_turns) when the sequence plays; so a later epoch outside the repeat subset is 1).
- The scorer labels the missing turn invalid with the reason incomplete_transcript when it graded fewer turns than planned, unless the sample stopped for context overflow (that keeps model_context_overflow). Old logs without turns_planned aren't flagged.
- report.py adds a coverage block (planned, completed, errored, and the id and epoch of each errored sample), a completion_rate row in the CSV, and a stderr warning. The exit status is unchanged. It reads inspect's own totals, so it also works with --no-log-samples.
- README: a "When samples fail" section, recommending --retry-on-error N.
Tests (IncompleteTranscriptTest, 7 tests): coverage with and without errors, the cut-off sample marked incomplete and left out of the concession rate, short runs not flagged, retry_on_error restarting a sample without duplicating or skipping a turn, coverage with --no-log-samples, and compatibility with a solver that records nothing. I wrote them first and they failed on the old code (for example 'hold>hold' != 'hold>hold>invalid'). run() in the test file now returns the error when a run fails, which was hidden behind a KeyError.
Checked: 59 inspect_eval, 60 eval/ and 2 scripts tests pass; with 20 samples at once the metadata isn't shared between epochs; the report works on an aborted run.
Not covered: a real provider (timeouts, 429/5xx), which is next. In an aborted run, samples that were still running are listed as errored (cancelled).
Reviewed the diff. This looks good to me and matches what we discussed in #114.
The turns_planned / incomplete_transcript handling addresses the main issue I was concerned about, and keeping coverage separate from the score while preserving the existing exit behavior makes sense. The tests also cover the important cases, especially cut-off transcripts not being counted in the concession rate and intentionally shorter runs not being flagged.
I'm fine with leaving real-provider 429/5xx/timeouts for the next pass as you noted. No blocking issues from me.
Thanks for the review @melover24 . I'll start on the provider-level failures (429/5xx, timeouts), eval-retry and the reproducibility check next.
