From ff6c66a74bc302d27ec2e5697f7060376f143882 Mon Sep 17 00:00:00 2001 From: PJ Date: Tue, 18 Aug 2026 14:08:59 +0530 Subject: [PATCH] fix(confusion-matrix): a campaign that died is missing data, not a true negative The sweep-level loop excluded a run on launch_error alone, while excludedBecause already checked the campaign process's exit code. An interrupted campaign wrote exit_code -1 with an empty launch_error, so its one completed seed scored the implementation as a clean cell on a tenth of the planned evidence. The fixture builder wrote one exit code into both the sweep record and the campaign run record, which is why no test could tell the two levels apart. --- .../confusion-matrix/checker.go | 20 +++++++++++-- .../confusion-matrix/fixtures_test.go | 10 +++++-- .../confusion-matrix/matrix_test.go | 30 +++++++++++++++++++ 3 files changed, 55 insertions(+), 5 deletions(-) diff --git a/cmd/internal-tools/confusion-matrix/checker.go b/cmd/internal-tools/confusion-matrix/checker.go index c0ffabf..7ca9a5e 100644 --- a/cmd/internal-tools/confusion-matrix/checker.go +++ b/cmd/internal-tools/confusion-matrix/checker.go @@ -170,8 +170,8 @@ func readImplementation(sweepDirectory string, record sweepImplementationRecord) fired := map[string]bool{} for _, run := range record.Runs { verdict.RunsRecorded++ - if run.LaunchError != "" { - verdict.ExcludedByReason[reasonLaunchError]++ + if reason := sweepRunExcludedBecause(run); reason != "" { + verdict.ExcludedByReason[reason]++ continue } directory := resolveCampaignDirectory(sweepDirectory, record.Name, run) @@ -254,6 +254,22 @@ func readCampaignRuns(directory string) ([]campaignRunRecord, error) { return records, nil } +// sweepRunExcludedBecause reads the outcome of the campaign process itself, +// which the records inside its directory cannot report. One sweep run is one +// campaign of one seed, so a campaign that died left a runs.jsonl that is +// partial or empty, and scoring the runs it did write reads the seeds it never +// reached as agreement. +func sweepRunExcludedBecause(record sweepRunRecord) string { + switch { + case record.LaunchError != "": + return reasonLaunchError + case record.ExitCode != 0: + return reasonNonzeroExit + default: + return "" + } +} + func excludedBecause(record campaignRunRecord) string { switch { case record.LaunchError != "": diff --git a/cmd/internal-tools/confusion-matrix/fixtures_test.go b/cmd/internal-tools/confusion-matrix/fixtures_test.go index 780eb34..76e8e11 100644 --- a/cmd/internal-tools/confusion-matrix/fixtures_test.go +++ b/cmd/internal-tools/confusion-matrix/fixtures_test.go @@ -16,8 +16,12 @@ import ( type fixtureRun struct { Seed int64 ExitCode int - TimedOut bool - Violated []string + // CampaignExitCode is the campaign process's own exit status, which the + // sweep records beside the campaign directory. It is not the exit status of + // the run inside that campaign. + CampaignExitCode int + TimedOut bool + Violated []string // Surfaces is the locatableSurfaces reading the trace records. A nil map // with NoTrace false still writes a reading of every surface false. Surfaces map[string]bool @@ -141,7 +145,7 @@ func writeImplementation(t *testing.T, built fixture, implementation fixtureImpl } runs = append(runs, map[string]any{ "seed": run.Seed, - "exit_code": run.ExitCode, + "exit_code": run.CampaignExitCode, "campaign_directory": campaignDirectory, }) } diff --git a/cmd/internal-tools/confusion-matrix/matrix_test.go b/cmd/internal-tools/confusion-matrix/matrix_test.go index 1748d97..fae3852 100644 --- a/cmd/internal-tools/confusion-matrix/matrix_test.go +++ b/cmd/internal-tools/confusion-matrix/matrix_test.go @@ -122,6 +122,36 @@ func TestCrossTabulateScoresEachImplementationIntoOneCell(t *testing.T) { } } +// TestACampaignThatDiedIsMissingDataNotATrueNegative covers the sweep it was +// interrupted on: the one seed the campaign got through wrote a clean run +// before the process died, and scoring the implementation on it reads the nine +// seeds that never ran as agreement between the checker and the reviewer. +func TestACampaignThatDiedIsMissingDataNotATrueNegative(t *testing.T) { + interrupted := cleanRun(1) + interrupted.CampaignExitCode = -1 + + emitted, stdout := runTool(t, writeFixture(t, []fixtureImplementation{{ + Name: "impl-07", Model: "Opus 5", + Runs: []fixtureRun{interrupted}, + Review: &fixtureReview{Overall: overallNotDefective}, + }}, defaultMapping)) + + entry := exclusionFor(t, emitted, "impl-07") + if entry.Reason != missingNoUsableRun { + t.Fatalf("impl-07 excluded as %q, want %q", entry.Reason, missingNoUsableRun) + } + if !strings.Contains(entry.Detail, reasonNonzeroExit) { + t.Errorf("exclusion detail %q does not name %q", entry.Detail, reasonNonzeroExit) + } + if emitted.Implementations.TrueNegative != 0 || emitted.Implementations.Scored != 0 { + t.Errorf("implementation matrix = %+v, want no cell: a dead campaign is absent evidence", + emitted.Implementations) + } + if !strings.Contains(stdout, "carry no cell") { + t.Errorf("the report never separates the dead campaign from the matrix:\n%s", stdout) + } +} + // TestUnlocatableSurfaceIsNeitherAPositiveNorANegative pins the rule the // pre-registration turns on: a clause whose every covering property a // never-located surface left unrunnable is a portability miss. Counting it as a