fix(analyze): censor a clean run at the steps it ran, and refuse mismatched budgets

A run stops at whichever comes first, the step budget or --duration, so
a clean run that reached the wall clock exited with fewer steps than the
budget and was still credited with the whole of it. The model arm pays a
network call and a screenshot per step, so it reaches the wall sooner and
was handed exposure it never had.

Nothing checked that two arms shared a budget either. Thirty identical
clean runs under budgets of 400 and 100 read a12 0.000 and p 1.685e-14
from the rank-sum while the log-rank in the same report read p 1.0000.
groupArms already refused this within one arm.

The claims the old convention left in comments and report lines are
corrected rather than left standing beside the new behaviour.

cmd/internal-tools/analyze/analysis.go

@@ -67,22 +67,28 @@ type analysis struct {
 	Notes          []string          `json:"notes,omitempty"`
 }
 
-const outcomeDescription = "steps to first violation, right-censored at the step budget"
+const outcomeDescription = "steps to first violation, right-censored at the last step a clean run reached"
 
-func analyse(arms []arm, now time.Time) analysis {
-	result, testable := baseAnalysis(arms, now)
+func analyse(arms []arm, now time.Time) (analysis, error) {
+	result, testable, err := baseAnalysis(arms, now)
+	if err != nil {
+		return analysis{}, err
+	}
 	if len(testable) >= 2 {
 		result.Pairwise = comparePairs(testable)
 		result.HolmFamilySize = countCorrected(result.Pairwise)
 	}
-	return result
+	return result, nil
 }
 
 // analysePaired is the seed-matched design of the actuation ablation: two arms
 // running the same seeds, contrasted seed by seed rather than as two
 // independent samples.
 func analysePaired(arms []arm, now time.Time) (analysis, error) {
-	result, testable := baseAnalysis(arms, now)
+	result, testable, err := baseAnalysis(arms, now)
+	if err != nil {
+		return analysis{}, err
+	}
 	if len(testable) != 2 {
 		return analysis{}, fmt.Errorf("a paired comparison needs exactly two arms with usable runs, found %d", len(testable))
 	}
@@ -102,7 +108,7 @@ func analysePaired(arms []arm, now time.Time) (analysis, error) {
 	return result, nil
 }
 
-func baseAnalysis(arms []arm, now time.Time) (analysis, []arm) {
+func baseAnalysis(arms []arm, now time.Time) (analysis, []arm, error) {
 	result := analysis{GeneratedAt: now, Outcome: outcomeDescription}
 	for _, current := range arms {
 		result.Arms = append(result.Arms, summarize(current))
@@ -118,6 +124,9 @@ func baseAnalysis(arms []arm, now time.Time) (analysis, []arm) {
 		result.Notes = append(result.Notes,
 			"arms with no usable runs are reported but left out of the log-rank test and the pairwise comparisons")
 	}
+	if err := sameBudget(testable); err != nil {
+		return analysis{}, nil, err
+	}
 	if len(testable) >= 2 {
 		names := make([]string, len(testable))
 		groups := make([][]observation, len(testable))
@@ -128,7 +137,24 @@ func baseAnalysis(arms []arm, now time.Time) (analysis, []arm) {
 		test := logRank(names, groups)
 		result.LogRank = &test
 	}
-	return result, testable
+	return result, testable, nil
+}
+
+// sameBudget refuses arms that were given different exposure. A clean run is
+// censored somewhere at or below its arm's budget, so the arm with the larger
+// budget carries censored runs the smaller arm could not have produced, and
+// every test that ranks the two against each other reads that as the arm
+// surviving longer. It is the cross-arm form of what groupArms already refuses
+// within one arm.
+func sameBudget(arms []arm) error {
+	for index := 1; index < len(arms); index++ {
+		if arms[index].Budget != arms[0].Budget {
+			return fmt.Errorf("arm %q has step budget %d and arm %q has %d: "+
+				"runs censored at different budgets cannot be compared",
+				arms[0].Name, arms[0].Budget, arms[index].Name, arms[index].Budget)
+		}
+	}
+	return nil
 }
 
 func countCorrected(pairs []pairwiseResult) int {

cmd/internal-tools/analyze/analysis_test.go

@@ -1,8 +1,10 @@
 package main
 
 import (
+	"io"
 	"math"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 )
@@ -227,11 +229,14 @@ func TestAnalyse_ExcludedRunsNeverBecomeObservations(t *testing.T) {
 }
 
 func TestAnalyse_ArmWithNoUsableRunsIsReportedButNotTested(t *testing.T) {
-	result := analyse([]arm{
+	result, err := analyse([]arm{
 		{Name: "a", Budget: 30, Runs: []classifiedRun{violatingRun(1, 4, 4), violatingRun(2, 6, 6)}},
 		{Name: "b", Budget: 30, Runs: []classifiedRun{cleanRun(1, 30), cleanRun(2, 30)}},
 		{Name: "c", Budget: 30, Runs: []classifiedRun{{Seed: 1, ExcludedBecause: reasonNonzeroExit}}},
 	}, time.Unix(0, 0).UTC())
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if len(result.Arms) != 3 {
 		t.Fatalf("%d arms reported, want all 3", len(result.Arms))
@@ -250,9 +255,12 @@ func TestAnalyse_ArmWithNoUsableRunsIsReportedButNotTested(t *testing.T) {
 // With a single testable arm there is nothing to compare against, and the tool
 // must say so instead of producing a statistic.
 func TestAnalyse_SingleArmHasNoTests(t *testing.T) {
-	result := analyse([]arm{
+	result, err := analyse([]arm{
 		{Name: "a", Budget: 30, Runs: []classifiedRun{violatingRun(1, 4, 4)}},
 	}, time.Unix(0, 0).UTC())
+	if err != nil {
+		t.Fatal(err)
+	}
 	if result.LogRank != nil || len(result.Pairwise) != 0 {
 		t.Errorf("log-rank %+v pairwise %v, want neither", result.LogRank, result.Pairwise)
 	}
@@ -297,6 +305,53 @@ func TestComparePairs_A12DirectionFollowsStepCounts(t *testing.T) {
 	}
 }
 
+func writeCleanCampaign(t *testing.T, directory, name string, budget, steps, runs int) {
+	t.Helper()
+	seeds := make([]int, 0, runs)
+	records := make([]map[string]any, 0, runs)
+	for seed := 1; seed <= runs; seed++ {
+		seeds = append(seeds, seed)
+		records = append(records, map[string]any{
+			"seed": seed, "exit_code": 0, "steps": steps, "actions": steps, "monotonic_millis": 60_000,
+		})
+	}
+	writeCampaign(t, directory, map[string]any{"arm": name, "max_steps": budget, "seeds": seeds}, records)
+}
+
+// Arms censored at different budgets are not on the same clock: every clean run
+// of the wider arm outranks every clean run of the narrower one whatever the
+// app did, so the rank-sum and the paired test reach a foregone conclusion the
+// log-rank in the same report contradicts. groupArms already refuses this
+// within one arm, and comparing across arms is the same hazard.
+func TestRun_RefusesToCompareArmsCensoredAtDifferentBudgets(t *testing.T) {
+	cases := []struct {
+		name      string
+		wideSteps int
+		arguments []string
+	}{
+		{name: "identical runs under different budgets", wideSteps: 100},
+		{name: "each arm run to its own budget", wideSteps: 400},
+		{name: "paired", wideSteps: 400, arguments: []string{"--paired"}},
+	}
+	for _, test := range cases {
+		root := t.TempDir()
+		wide := filepath.Join(root, "wide")
+		narrow := filepath.Join(root, "narrow")
+		writeCleanCampaign(t, wide, "wide", 400, test.wideSteps, 30)
+		writeCleanCampaign(t, narrow, "narrow", 100, 100, 30)
+
+		err := run(append(test.arguments, wide, narrow), io.Discard, io.Discard)
+		if err == nil {
+			t.Fatalf("%s: arms censored at 400 and at 100 steps were compared without complaint", test.name)
+		}
+		for _, fragment := range []string{"wide", "400", "narrow", "100", "different budgets"} {
+			if !strings.Contains(err.Error(), fragment) {
+				t.Errorf("%s: error %q is missing %q", test.name, err, fragment)
+			}
+		}
+	}
+}
+
 func manyRuns(count, originStep int) []classifiedRun {
 	runs := make([]classifiedRun, 0, count)
 	for index := 0; index < count; index++ {

cmd/internal-tools/analyze/end_to_end_test.go

@@ -90,7 +90,7 @@ func TestRun_EndToEndOverFixtureCampaignDirectories(t *testing.T) {
 
 	text := stdout.String()
 	for _, fragment := range []string{
-		"steps to first violation, right-censored at the step budget",
+		"steps to first violation, right-censored at the last step a clean run reached",
 		"log-rank across 2 arms",
 		"pairwise wilcoxon rank-sum",
 		"llm vs seeded",

cmd/internal-tools/analyze/load.go

@@ -17,8 +17,8 @@ const (
 )
 
 // manifest mirrors the fields analyze reads from campaign.json. The step budget
-// lives here rather than in any run, because it is what clean runs are censored
-// at and every run in an arm has to share it.
+// lives here rather than in any run, because it is the exposure every run in an
+// arm was given and the ceiling a clean run can be censored at.
 type manifest struct {
 	Arm       string  `json:"arm"`
 	Generator string  `json:"generator"`
@@ -60,8 +60,8 @@ type runRecord struct {
 }
 
 // Exclusion reasons. A run that failed or timed out is missing data, not a
-// censored observation: it never ran its budget, so treating it as a clean run
-// that survived to the budget would bias the survival estimate downward.
+// censored observation: it broke off, so its step count is not exposure the app
+// survived and counting it as one would bias the survival estimate downward.
 const (
 	reasonLaunchError   = "launch error"
 	reasonTimedOut      = "timed out"
@@ -209,8 +209,8 @@ func classify(record runRecord, budget int) classifiedRun {
 // run ends, and timing that at the step that armed it would record a liveness
 // failure flushed at the budget as a violation found on the first step, which
 // is a number the run cannot support and which no censored run can be compared
-// against: the budget is the clock the clean runs are censored on, so the events
-// have to be on it too. A campaign written before the field existed carries no
+// against: the end of the run is the clock the clean runs are censored on, so
+// the events have to be on it too. A campaign written before the field existed carries no
 // detected step and keeps the origin.
 func eventStep(record runRecord, origin int) int {
 	if record.FirstViolationDetectedStep == nil {
@@ -271,7 +271,8 @@ func groupArms(directories []string) ([]arm, error) {
 }
 
 // observations returns the usable runs as survival data: an event at the step
-// that armed the first violation, or a censored observation at the step budget.
+// that armed the first violation, or a censored observation at the last step
+// the run reached.
 func (a arm) observations() []observation {
 	var result []observation
 	for _, item := range a.Runs {
@@ -287,13 +288,15 @@ func observationOf(item classifiedRun, budget int) observation {
 	if item.Violated {
 		return observation{Steps: float64(item.EventStep), Event: true}
 	}
-	return observation{Steps: float64(budget), Event: false}
+	// A run that hit the campaign's wall clock stopped short of the budget, and
+	// the steps it never ran are not exposure it survived.
+	return observation{Steps: float64(min(item.Steps, budget)), Event: false}
 }
 
 // stepTimes is the observations flattened to plain numbers, censored runs held
-// at the budget. Holding them there rather than dropping them is conservative:
-// it can only understate how much sooner a violating arm finds its first
-// defect, never overstate it.
+// at the steps they ran. Holding them there rather than dropping them is
+// conservative: it can only understate how much sooner a violating arm finds
+// its first defect, never overstate it.
 func (a arm) stepTimes() []float64 {
 	var result []float64
 	for _, item := range a.observations() {

cmd/internal-tools/analyze/load_test.go

@@ -31,18 +31,32 @@ func TestClassify_FailedAndTimedOutRunsAreMissingDataNotCensored(t *testing.T) {
 	}
 }
 
-func TestClassify_CleanRunIsCensoredAtTheBudget(t *testing.T) {
-	item := classify(runRecord{Seed: 4, Steps: 50, DurationMillis: 1000}, 50)
-	if item.ExcludedBecause != "" {
-		t.Fatalf("excluded because %q", item.ExcludedBecause)
+// A run also ends when the campaign's wall clock does, so a clean run can stop
+// well short of the budget. Censoring it at the budget would credit it with
+// steps it never ran, and a slower arm loses fewer steps in the same wall clock
+// than a fast one, so the credit does not cancel between arms.
+func TestClassify_CleanRunIsCensoredAtTheStepsItRan(t *testing.T) {
+	cases := []struct {
+		name     string
+		steps    int
+		budget   int
+		censored float64
+	}{
+		{"stopped by the wall clock short of the budget", 12, 400, 12},
+		{"ran the whole budget", 400, 400, 400},
+		{"recorded more steps than the manifest budget", 420, 400, 400},
 	}
-	if item.Violated {
-		t.Error("clean run marked as violated")
-	}
-	current := arm{Budget: 50, Runs: []classifiedRun{item}}
-	observations := current.observations()
-	if len(observations) != 1 || observations[0].Event || observations[0].Steps != 50 {
-		t.Errorf("observations %+v, want one censored observation at 50", observations)
+	for _, test := range cases {
+		item := classify(runRecord{Seed: 4, Steps: test.steps, DurationMillis: 1000}, test.budget)
+		if item.ExcludedBecause != "" || item.Violated {
+			t.Fatalf("%s: run %+v, want a usable clean run", test.name, item)
+		}
+		current := arm{Budget: test.budget, Runs: []classifiedRun{item}}
+		observations := current.observations()
+		if len(observations) != 1 || observations[0].Event || observations[0].Steps != test.censored {
+			t.Errorf("%s: observations %+v, want one censored observation at %v",
+				test.name, observations, test.censored)
+		}
 	}
 }
 

cmd/internal-tools/analyze/main.go

@@ -1,6 +1,6 @@
 // Command analyze reduces campaign directories to the statistics the
 // evaluation reports. The primary outcome is steps to first violation, with
-// clean runs right-censored at the step budget rather than discarded: defect
+// clean runs right-censored where they stopped rather than discarded: defect
 // yield per run is a binary that would need on the order of eighty runs an arm
 // to separate, while survival analysis uses every run, including the clean ones.
 package main
@@ -24,7 +24,7 @@ Usage:
 
 Each directory is one produced by the campaign tool and must hold campaign.json
 and runs.jsonl. Directories sharing an arm label are pooled and must agree on
-the step budget.
+the step budget, and arms compared against each other must agree on it too.
 
 One invocation is one research question: Holm corrects across the comparisons it
 produces and across nothing else.
@@ -87,7 +87,10 @@ func run(arguments []string, stdout, stderr io.Writer) error {
 			return err
 		}
 	} else {
-		result = analyse(arms, time.Now().UTC())
+		result, err = analyse(arms, time.Now().UTC())
+		if err != nil {
+			return err
+		}
 	}
 	result.Question = question
 	writeReport(result, stdout)

cmd/internal-tools/analyze/paired.go

@@ -174,7 +174,7 @@ type pairedComparison struct {
 }
 
 // pairArms matches the two arms by seed and contrasts them pair by pair.
-// Censored runs enter at the step budget, the same convention the unpaired
+// Censored runs enter at the steps they ran, the same convention the unpaired
 // comparison uses. A seed usable in one arm and not the other is named rather
 // than dropped silently, because that is a host that lost a run and it is what
 // the campaign manifest exists to make visible.

cmd/internal-tools/analyze/ranksum_test.go

@@ -58,7 +58,7 @@ func TestVargaDelaneyA12_BoundaryCases(t *testing.T) {
 
 // The counting definition and the rank-sum route must agree, including when the
 // samples are tied against each other, which is the case the evaluation data is
-// always in because censored runs are all held at the budget.
+// usually in because censored runs pile up on the step they stopped at.
 func TestVargaDelaneyA12_AgreesWithRankSumStatistic(t *testing.T) {
 	cases := [][2][]float64{
 		{{1, 2, 3}, {2, 3, 4}},

cmd/internal-tools/analyze/report.go

@@ -99,7 +99,7 @@ func writeReport(result analysis, out io.Writer) {
 	}
 
 	if len(result.Pairwise) > 0 {
-		fmt.Fprintln(out, "\npairwise wilcoxon rank-sum, censored runs held at the budget")
+		fmt.Fprintln(out, "\npairwise wilcoxon rank-sum, censored runs held at the steps they ran")
 		fmt.Fprintln(out, "a12 above 0.5 means the first arm takes more steps to its first violation")
 		writeTable(out, []string{"comparison", "n1", "n2", "u", "a12", "p", "holm p"}, func(add func(...string)) {
 			for _, pair := range result.Pairwise {
@@ -130,7 +130,7 @@ func writeReport(result analysis, out io.Writer) {
 }
 
 func writePaired(out io.Writer, comparison pairedComparison) {
-	fmt.Fprintf(out, "\npaired per-seed difference, %s minus %s, censored runs held at the budget\n",
+	fmt.Fprintf(out, "\npaired per-seed difference, %s minus %s, censored runs held at the steps they ran\n",
 		comparison.First, comparison.Second)
 	fmt.Fprintf(out, "%d seed pair(s): %s sooner in %d, %s sooner in %d, tied in %d\n",
 		comparison.Pairs, comparison.First, comparison.FirstSooner,

cmd/internal-tools/analyze/survival.go

@@ -7,8 +7,7 @@ import (
 
 // observation is one run reduced to what the survival analysis needs: the step
 // count at which it left the risk set, and whether it left because a violation
-// was found (an event) or because it exhausted the step budget without one
-// (right-censored). A run that failed or timed out is neither and never
+// was found (an event) or because the run ended without one (right-censored). A run that failed or timed out is neither and never
 // reaches this type.
 type observation struct {
 	Steps float64