diff --git a/cmd/internal-tools/analyze/load.go b/cmd/internal-tools/analyze/load.go index 87374fa..7b8d823 100644 --- a/cmd/internal-tools/analyze/load.go +++ b/cmd/internal-tools/analyze/load.go @@ -52,10 +52,12 @@ type runRecord struct { // It is what the survival analysis times the event by; see eventStep. FirstViolationDetectedStep *int `json:"first_violation_detected_step"` ViolatedProperties []string `json:"violated_properties"` - // Actions is the count of steps that dispatched an action, and it is a - // pointer so that a runs.jsonl written before the campaign tool counted - // them is refused rather than read as an arm that acted zero times. The - // campaign tool always emits the field, so its absence dates the file. + // Actions is the count of steps on which the action generator dispatched an + // action, which excludes the steps the spec's setup drove before the + // generator was consulted wherever the trace names them. It is a pointer so + // that a runs.jsonl written before the campaign tool counted them is refused + // rather than read as an arm that acted zero times. The campaign tool always + // emits the field, so its absence dates the file. Actions *int `json:"actions"` } diff --git a/cmd/internal-tools/analyze/report.go b/cmd/internal-tools/analyze/report.go index 57e9bdb..dfa6773 100644 --- a/cmd/internal-tools/analyze/report.go +++ b/cmd/internal-tools/analyze/report.go @@ -34,7 +34,8 @@ func writeReport(result analysis, out io.Writer) { fmt.Fprintln(out) fmt.Fprintln(out, "a detection is one distinct property violated in one run; run hours sum the time the runs worked,") fmt.Fprintln(out, "on the monotonic clock, so a host that slept mid-run is not charged for the sleep") - fmt.Fprintln(out, "actions count the steps that dispatched one; the rest chose nothing or had the choice thrown away") + fmt.Fprintln(out, "actions count the steps the generator dispatched one on; the rest chose nothing, had the choice") + fmt.Fprintln(out, "thrown away, or were the spec's setup driving the app into position before the generator ran") writeTable(out, []string{"arm", "steps", "actions", "run hours", "detections", "defects/1k actions", "defects/hour", "distinct defects", "found in one run"}, func(add func(...string)) { for _, summary := range result.Arms { diff --git a/cmd/internal-tools/campaign/summary.go b/cmd/internal-tools/campaign/summary.go index dcbdc3d..5de1490 100644 --- a/cmd/internal-tools/campaign/summary.go +++ b/cmd/internal-tools/campaign/summary.go @@ -20,11 +20,13 @@ const maxTraceLineBytes = 16 * 1024 * 1024 // to open trace.jsonl again. type traceSummary struct { 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 counts only the steps on which the action generator both chose an + // action and dispatched it. A step whose policy declined to act, a step + // whose chosen action the runner threw away, and a step the spec's setup + // drove before the generator was ever consulted all explored nothing, so + // counting any of them 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"` @@ -45,20 +47,39 @@ type traceLine struct { // 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"` - // NextAction is read only for its presence: a step that chose no action - // carries none at all. - NextAction json.RawMessage `json:"next_action"` + // NextAction is nil on a step that chose no action. Its Source names the + // backend that chose the action, which is the only thing in the trace + // separating a step the spec's setup drove from one the generator drove. + NextAction *actionLine `json:"next_action"` ActionSkipped string `json:"action_skipped"` PreconditionFailure string `json:"precondition_failure"` Violations []string `json:"violations"` Witnesses map[string]trace.Witness `json:"witnesses"` } -func dispatchedAction(line traceLine) bool { - if line.ActionSkipped != "" { +type actionLine struct { + Source string `json:"source"` +} + +// generatorDispatched reports whether this step drove the app on the action +// generator's behalf, which is the exposure a per-action rate divides by. A +// spec's setup puts the app into its starting position before the generator is +// consulted, so its login taps are dispatched actions that explored nothing. +// +// Only a model run separates the two: the LLM backend stamps source="llm" on +// what it chose and leaves setup's actions unstamped. The seeded picker resolves +// setup precedence inside the one JS call it makes and stamps nothing at all, so +// its setup steps are indistinguishable from its generator steps in the trace +// and are counted; reading their absent source as setup would report every +// seeded run as having explored nothing. +func generatorDispatched(line traceLine, generator string) bool { + if line.ActionSkipped != "" || line.NextAction == nil { return false } - return len(line.NextAction) > 0 && string(line.NextAction) != "null" + if generator != "llm" { + return true + } + return line.NextAction.Source == "llm" } // findRunDirectory returns the run directory `sanderling test` created inside @@ -92,14 +113,34 @@ func summarizeRun(seedDirectory string) (string, traceSummary, error) { if err != nil { return "", traceSummary{}, err } - summary, err := summarizeTrace(filepath.Join(seedDirectory, name, "trace.jsonl")) + directory := filepath.Join(seedDirectory, name) + generator, err := runGenerator(directory) + if err != nil { + return name, traceSummary{}, err + } + summary, err := summarizeTrace(filepath.Join(directory, "trace.jsonl"), generator) if err != nil { return name, traceSummary{}, err } return name, summary, nil } -func summarizeTrace(tracePath string) (traceSummary, error) { +// runGenerator reads which picker drove the run. The trace on its own cannot +// say whether an unstamped action came from the seeded picker or from the +// spec's setup under the model picker, and the two count differently. +func runGenerator(runDirectory string) (string, error) { + body, err := os.ReadFile(filepath.Join(runDirectory, "meta.json")) + if err != nil { + return "", fmt.Errorf("read meta: %w", err) + } + var meta trace.Meta + if err := json.Unmarshal(body, &meta); err != nil { + return "", fmt.Errorf("decode meta: %w", err) + } + return meta.Generator, nil +} + +func summarizeTrace(tracePath, generator string) (traceSummary, error) { file, err := os.Open(tracePath) if err != nil { return traceSummary{}, fmt.Errorf("open trace: %w", err) @@ -127,7 +168,7 @@ func summarizeTrace(tracePath string) (traceSummary, error) { if !synthetic && line.Index > summary.Steps { summary.Steps = line.Index } - if !synthetic && dispatchedAction(line) { + if !synthetic && generatorDispatched(line, generator) { summary.Actions++ } if line.PreconditionFailure != "" { diff --git a/cmd/internal-tools/campaign/summary_test.go b/cmd/internal-tools/campaign/summary_test.go index aefa36b..56b8a62 100644 --- a/cmd/internal-tools/campaign/summary_test.go +++ b/cmd/internal-tools/campaign/summary_test.go @@ -34,13 +34,51 @@ func skippedActionStep(index int, reason string) trace.Step { return step } +func setupStep(index int) trace.Step { + step := observedStep(index) + step.NextAction = &trace.Action{ + Kind: "InputText", + Selector: "testTag:LoginScreen > testTag:LoginEmail", + Text: "demo@folio.app", + } + return step +} + +func modelStep(index int) trace.Step { + step := actingStep(index) + step.NextAction.Source = "llm" + return step +} + +func skippedModelStep(index int, reason string) trace.Step { + step := modelStep(index) + step.ActionSkipped = reason + return step +} + func writeRunDirectory(t *testing.T, seedDirectory, name string, steps []trace.Step) string { + t.Helper() + return writeRunDirectoryWithMeta( + t, + seedDirectory, + name, + trace.Meta{Seed: 11, Platform: "web", Arm: "seeded-baseline"}, + steps, + ) +} + +func writeRunDirectoryWithMeta( + t *testing.T, + seedDirectory, name string, + declared trace.Meta, + steps []trace.Step, +) string { t.Helper() directory := filepath.Join(seedDirectory, name) if err := os.MkdirAll(directory, 0o755); err != nil { t.Fatal(err) } - meta, err := json.Marshal(trace.Meta{Seed: 11, Platform: "web", Arm: "seeded-baseline"}) + meta, err := json.Marshal(declared) if err != nil { t.Fatal(err) } @@ -111,6 +149,68 @@ func TestSummarizeRun_CountsOnlyStepsThatDispatchedAnAction(t *testing.T) { } } +func TestSummarizeRun_ModelRunLeavesTheSetupsLoginOutOfTheActionCount(t *testing.T) { + seedDirectory := t.TempDir() + writeRunDirectoryWithMeta(t, seedDirectory, "20260812-090000", + trace.Meta{Seed: 11, Platform: "web", Arm: "llm-identifier", Generator: "llm"}, + []trace.Step{ + setupStep(1), + setupStep(2), + setupStep(3), + modelStep(4), + skippedModelStep(5, "unresolved_selector"), + modelStep(6), + }) + + _, summary, err := summarizeRun(seedDirectory) + if err != nil { + t.Fatal(err) + } + if summary.Steps != 6 { + t.Errorf("steps: got %d, want 6", summary.Steps) + } + if summary.Actions != 2 { + t.Errorf("actions: got %d, want 2 (three login steps were setup's and one generator action was thrown away)", summary.Actions) + } +} + +func TestSummarizeRun_ModelRunWithoutSetupCountsEveryDispatchedStep(t *testing.T) { + seedDirectory := t.TempDir() + writeRunDirectoryWithMeta(t, seedDirectory, "20260812-090000", + trace.Meta{Seed: 11, Platform: "web", Arm: "llm-identifier", Generator: "llm"}, + []trace.Step{modelStep(1), modelStep(2), modelStep(3), modelStep(4)}) + + _, summary, err := summarizeRun(seedDirectory) + if err != nil { + t.Fatal(err) + } + if summary.Actions != 4 { + t.Errorf("actions: got %d, want 4", summary.Actions) + } +} + +func TestSummarizeRun_SeededRunCountsSetupBecauseItsTraceCannotNameIt(t *testing.T) { + seedDirectory := t.TempDir() + writeRunDirectoryWithMeta(t, seedDirectory, "20260812-090000", + trace.Meta{Seed: 11, Platform: "web", Arm: "seeded-baseline", Generator: "seeded"}, + []trace.Step{ + setupStep(1), + setupStep(2), + setupStep(3), + actingStep(4), + actingStep(5), + }) + + _, summary, err := summarizeRun(seedDirectory) + if err != nil { + t.Fatal(err) + } + if summary.Actions != 5 { + t.Errorf("actions: got %d, want 5: the seeded picker stamps no source, so excluding "+ + "unstamped steps would count its whole run as setup", summary.Actions) + } +} + func TestSummarizeRun_SkipReasonsEachSuppressTheAction(t *testing.T) { for _, reason := range []string{ "no_target", "unresolved_selector", "missing_key",