From f6d562e3dcb436c1dae738406cb3cbe8f4a5971f Mon Sep 17 00:00:00 2001 From: PJ Date: Wed, 12 Aug 2026 23:43:19 +0530 Subject: [PATCH] feat(campaign): count dispatched actions, not steps A step where the policy declined has no action, and a step whose action was never dispatched did nothing. Both were being counted as actions by everything downstream. Claude-Session: https://claude.ai/code/session_01A5KmftdEJ49A9z5mF5ESrX --- cmd/internal-tools/campaign/campaign_test.go | 35 ++++++++ cmd/internal-tools/campaign/summary.go | 28 +++++- cmd/internal-tools/campaign/summary_test.go | 94 ++++++++++++++++++++ 3 files changed, 153 insertions(+), 4 deletions(-) diff --git a/cmd/internal-tools/campaign/campaign_test.go b/cmd/internal-tools/campaign/campaign_test.go index 1a1b224..23fbdcf 100644 --- a/cmd/internal-tools/campaign/campaign_test.go +++ b/cmd/internal-tools/campaign/campaign_test.go @@ -69,6 +69,41 @@ func writeFakeRun(t *testing.T, arguments []string, steps []trace.Step) { writeRunDirectory(t, argumentValue(arguments, "--output"), "20260101-000000", steps) } +func TestRunCampaign_RecordsDispatchedActionsNotSteps(t *testing.T) { + directory := t.TempDir() + configuration := testConfiguration(t, directory, "--seeds", "1") + + executor := versionAnswering(func(_ context.Context, _ string, arguments []string, _ io.Writer) (int, error) { + writeFakeRun(t, arguments, []trace.Step{ + actingStep(1), observedStep(2), skippedActionStep(3, "unresolved_selector"), actingStep(4), + }) + return 0, nil + }) + if err := runCampaign(context.Background(), configuration, executor, io.Discard); err != nil { + t.Fatal(err) + } + + records := readRecords(t, directory) + if len(records) != 1 { + t.Fatalf("records: got %d, want 1", len(records)) + } + if records[0].Steps != 4 || records[0].Actions != 2 { + t.Errorf("steps %d actions %d, want 4 and 2", records[0].Steps, records[0].Actions) + } +} + +// A run that never produced a readable trace still has to carry the field, so +// analysis can tell a zero-action run from a file written before the count. +func TestRunRecord_AlwaysCarriesTheActionCount(t *testing.T) { + body, err := json.Marshal(runRecord{Seed: 7, TraceError: "no run directory with meta.json"}) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(body), `"actions":0`) { + t.Errorf("record %s omits the action count", body) + } +} + func TestRunCampaign_WritesManifestBeforeAnyRun(t *testing.T) { directory := t.TempDir() configuration := testConfiguration(t, directory) diff --git a/cmd/internal-tools/campaign/summary.go b/cmd/internal-tools/campaign/summary.go index 2d5e442..351821d 100644 --- a/cmd/internal-tools/campaign/summary.go +++ b/cmd/internal-tools/campaign/summary.go @@ -19,7 +19,13 @@ const maxTraceLineBytes = 16 * 1024 * 1024 // traceSummary is everything the analysis needs from one run, so it never has // to open trace.jsonl again. type traceSummary struct { - Steps int `json:"steps"` + Steps int `json:"steps"` + // Actions counts only the steps that both chose an action and dispatched + // it. A step whose policy declined to act, and a step whose chosen action + // the runner threw away, changed nothing about the app, so counting either + // as an action inflates the denominator of every per-action rate. The + // inflation is policy-dependent, so it does not cancel between arms. + Actions int `json:"actions"` FirstViolationOriginStep *int `json:"first_violation_origin_step"` FirstViolationDetectedStep *int `json:"first_violation_detected_step"` FirstViolationProperties []string `json:"first_violation_properties,omitempty"` @@ -32,9 +38,20 @@ type traceLine struct { Index int `json:"step"` // Hierarchy is read only for its presence: the run-end finalize line is the // one line carrying violations without an observed hierarchy. - Hierarchy json.RawMessage `json:"hierarchy"` - Violations []string `json:"violations"` - Witnesses map[string]trace.Witness `json:"witnesses"` + Hierarchy json.RawMessage `json:"hierarchy"` + // NextAction is read only for its presence: a step that chose no action + // carries none at all. + NextAction json.RawMessage `json:"next_action"` + ActionSkipped string `json:"action_skipped"` + Violations []string `json:"violations"` + Witnesses map[string]trace.Witness `json:"witnesses"` +} + +func dispatchedAction(line traceLine) bool { + if line.ActionSkipped != "" { + return false + } + return len(line.NextAction) > 0 && string(line.NextAction) != "null" } // findRunDirectory returns the run directory `sanderling test` created inside @@ -103,6 +120,9 @@ func summarizeTrace(tracePath string) (traceSummary, error) { if !synthetic && line.Index > summary.Steps { summary.Steps = line.Index } + if !synthetic && dispatchedAction(line) { + summary.Actions++ + } for _, property := range line.Violations { violated[property] = true recordViolation(&summary, line, property) diff --git a/cmd/internal-tools/campaign/summary_test.go b/cmd/internal-tools/campaign/summary_test.go index 5e43ab1..de609ac 100644 --- a/cmd/internal-tools/campaign/summary_test.go +++ b/cmd/internal-tools/campaign/summary_test.go @@ -22,6 +22,18 @@ func observedStep(index int) trace.Step { } } +func actingStep(index int) trace.Step { + step := observedStep(index) + step.NextAction = &trace.Action{Kind: "tap", X: 12, Y: 34} + return step +} + +func skippedActionStep(index int, reason string) trace.Step { + step := actingStep(index) + step.ActionSkipped = reason + return step +} + func writeRunDirectory(t *testing.T, seedDirectory, name string, steps []trace.Step) string { t.Helper() directory := filepath.Join(seedDirectory, name) @@ -72,6 +84,88 @@ func TestSummarizeRun_CleanRunIsCensored(t *testing.T) { } } +func TestSummarizeRun_CountsOnlyStepsThatDispatchedAnAction(t *testing.T) { + seedDirectory := t.TempDir() + writeRunDirectory(t, seedDirectory, "20260812-090000", []trace.Step{ + actingStep(1), + observedStep(2), + actingStep(3), + skippedActionStep(4, "no_target"), + skippedActionStep(5, "unresolved_selector"), + skippedActionStep(6, "missing_key"), + skippedActionStep(7, "zero_duration_wait"), + skippedActionStep(8, "app_left_foreground"), + skippedActionStep(9, "apply_error"), + actingStep(10), + }) + + _, summary, err := summarizeRun(seedDirectory) + if err != nil { + t.Fatal(err) + } + if summary.Steps != 10 { + t.Errorf("steps: got %d, want 10", summary.Steps) + } + if summary.Actions != 3 { + t.Errorf("actions: got %d, want 3 (one step chose nothing and six were never dispatched)", summary.Actions) + } +} + +func TestSummarizeRun_SkipReasonsEachSuppressTheAction(t *testing.T) { + for _, reason := range []string{ + "no_target", "unresolved_selector", "missing_key", + "zero_duration_wait", "app_left_foreground", "apply_error", + } { + seedDirectory := t.TempDir() + writeRunDirectory(t, seedDirectory, "20260812-090000", []trace.Step{ + actingStep(1), skippedActionStep(2, reason), + }) + _, summary, err := summarizeRun(seedDirectory) + if err != nil { + t.Fatal(err) + } + if summary.Actions != 1 { + t.Errorf("%s: actions got %d, want 1", reason, summary.Actions) + } + } +} + +func TestSummarizeTrace_NullActionIsNoAction(t *testing.T) { + seedDirectory := t.TempDir() + directory := writeRunDirectory(t, seedDirectory, "20260812-090000", []trace.Step{observedStep(1)}) + lines := "{\"step\":1,\"hierarchy\":{},\"next_action\":null}\n" + + "{\"step\":2,\"hierarchy\":{},\"next_action\":{\"kind\":\"tap\"},\"action_skipped\":\"\"}\n" + if err := os.WriteFile(filepath.Join(directory, "trace.jsonl"), []byte(lines), 0o644); err != nil { + t.Fatal(err) + } + _, summary, err := summarizeRun(seedDirectory) + if err != nil { + t.Fatal(err) + } + if summary.Actions != 1 { + t.Errorf("actions: got %d, want 1", summary.Actions) + } +} + +func TestSummarizeRun_FinalizeLineIsNotAnAction(t *testing.T) { + seedDirectory := t.TempDir() + finalize := trace.Step{ + Index: 3, + Timestamp: time.Now().UTC(), + NextAction: &trace.Action{Kind: "tap"}, + Violations: []string{"eventuallySettles"}, + } + writeRunDirectory(t, seedDirectory, "20260812-090000", []trace.Step{actingStep(1), actingStep(2), finalize}) + + _, summary, err := summarizeRun(seedDirectory) + if err != nil { + t.Fatal(err) + } + if summary.Actions != 2 { + t.Errorf("actions: got %d, want 2", summary.Actions) + } +} + func TestSummarizeRun_UsesWitnessOriginNotDetectionStep(t *testing.T) { seedDirectory := t.TempDir() violating := observedStep(9)