From 454988fbc8a7d2c952788fcbc5983466c22f5c2e Mon Sep 17 00:00:00 2001 From: PJ Date: Tue, 18 Aug 2026 17:58:38 +0530 Subject: [PATCH] fix(trace): an action names the generator that produced it The setup exclusion landed for the model arm only, because only a model pick stamped a source. A seeded run returned setup's action through the same entry with no marker, so its denominator still counted the login while the model arm's did not, and the two are compared. serializeAction names setup and seeded on the wire, so both arms are counted by one rule. An already-recorded trace names nothing and keeps exactly the count it was reported with; unattributed_actions counts those steps so the old denominator cannot pass as the new one. TraceVersion is deliberately unbumped: oracle-reduction refuses a differing version, and a bump would make all 169 recorded runs unreplayable. --- cmd/internal-tools/analyze/analysis.go | 7 +- cmd/internal-tools/campaign/summary.go | 44 ++++++++---- cmd/internal-tools/campaign/summary_test.go | 77 +++++++++++++++++++-- docs/manual/runs.md | 2 + docs/manual/spec-language.md | 2 + internal/runner/llm_source.go | 29 ++++---- internal/runner/llm_source_test.go | 6 +- internal/runner/runner.go | 14 ++-- internal/runner/runner_test.go | 56 +++++++++++++++ internal/runner/testdata/trace.jsonl | 8 +-- internal/trace/writer.go | 14 +++- internal/verifier/marshal.go | 13 ++++ internal/verifier/policy_parity_test.go | 7 +- internal/verifier/setup_action_test.go | 52 ++++++++++++++ internal/verifier/types.go | 4 ++ pkg/spec/src/runtime-entry.ts | 30 ++++++-- pkg/spec/test/fixtures/parity-golden.json | 60 ++++++++++------ pkg/spec/test/parity-harness.ts | 6 +- pkg/spec/test/web-runtime.test.ts | 28 ++++++++ 19 files changed, 377 insertions(+), 82 deletions(-) diff --git a/cmd/internal-tools/analyze/analysis.go b/cmd/internal-tools/analyze/analysis.go index c6b9d5d..ff33124 100644 --- a/cmd/internal-tools/analyze/analysis.go +++ b/cmd/internal-tools/analyze/analysis.go @@ -225,9 +225,10 @@ func summarize(current arm) armSummary { } summary.Usable++ summary.TotalSteps += item.Steps - // Steps and actions differ by the steps that chose no action and the - // steps whose action was never dispatched. Only dispatched actions - // exercised the app, so only they belong in a per-action rate. + // Steps and actions differ by the steps that chose no action, the steps + // whose action was never dispatched, and the steps the spec's setup + // drove into position. Only what the action generator dispatched + // explored the app, so only that belongs in a per-action rate. summary.TotalActions += item.Actions summary.TotalRunHours += float64(item.MonotonicMillis) / float64(time.Hour/time.Millisecond) if item.ClampedToBudget { diff --git a/cmd/internal-tools/campaign/summary.go b/cmd/internal-tools/campaign/summary.go index 5de1490..cbdf5aa 100644 --- a/cmd/internal-tools/campaign/summary.go +++ b/cmd/internal-tools/campaign/summary.go @@ -27,7 +27,12 @@ type traceSummary struct { // 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"` + Actions int `json:"actions"` + // UnattributedActions counts the dispatched steps whose action names no + // producer, which only a trace recorded before actions carried one can do. + // Such a run's Actions is the count it was already reported with rather than + // a setup-excluding one, and this is what says so. + UnattributedActions int `json:"unattributed_actions,omitempty"` FirstViolationOriginStep *int `json:"first_violation_origin_step"` FirstViolationDetectedStep *int `json:"first_violation_detected_step"` FirstViolationProperties []string `json:"first_violation_properties,omitempty"` @@ -65,21 +70,32 @@ type actionLine struct { // 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. +// Both arms are read the same way, off the source the action names, because a +// denominator that excludes setup on one arm and includes it on the other makes +// the two rates incomparable. // -// 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. +// An action naming no source at all is one recorded before the distinction +// existed and cannot be attributed now, so each arm keeps the count it was +// already reported with: everything the seeded picker dispatched, and only what +// the model stamped. summarizeTrace counts those steps separately so a +// pre-source run cannot pass its denominator off as a setup-excluding one. func generatorDispatched(line traceLine, generator string) bool { if line.ActionSkipped != "" || line.NextAction == nil { return false } - if generator != "llm" { + switch line.NextAction.Source { + case "": + return generator != trace.ActionSourceModel + case trace.ActionSourceSetup: + return false + default: return true } - return line.NextAction.Source == "llm" +} + +// actionUnattributed reports a dispatched step whose action names no producer. +func actionUnattributed(line traceLine) bool { + return line.ActionSkipped == "" && line.NextAction != nil && line.NextAction.Source == "" } // findRunDirectory returns the run directory `sanderling test` created inside @@ -125,9 +141,10 @@ func summarizeRun(seedDirectory string) (string, traceSummary, error) { return name, summary, nil } -// 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. +// runGenerator reads which picker drove the run. A trace recorded before +// actions named their source 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 { @@ -171,6 +188,9 @@ func summarizeTrace(tracePath, generator string) (traceSummary, error) { if !synthetic && generatorDispatched(line, generator) { summary.Actions++ } + if !synthetic && actionUnattributed(line) { + summary.UnattributedActions++ + } if line.PreconditionFailure != "" { summary.PreconditionFailures++ } diff --git a/cmd/internal-tools/campaign/summary_test.go b/cmd/internal-tools/campaign/summary_test.go index 56b8a62..8a5b0c2 100644 --- a/cmd/internal-tools/campaign/summary_test.go +++ b/cmd/internal-tools/campaign/summary_test.go @@ -40,13 +40,20 @@ func setupStep(index int) trace.Step { Kind: "InputText", Selector: "testTag:LoginScreen > testTag:LoginEmail", Text: "demo@folio.app", + Source: trace.ActionSourceSetup, } return step } +func seededStep(index int) trace.Step { + step := actingStep(index) + step.NextAction.Source = trace.ActionSourceSeeded + return step +} + func modelStep(index int) trace.Step { step := actingStep(index) - step.NextAction.Source = "llm" + step.NextAction.Source = trace.ActionSourceModel return step } @@ -189,7 +196,7 @@ func TestSummarizeRun_ModelRunWithoutSetupCountsEveryDispatchedStep(t *testing.T } } -func TestSummarizeRun_SeededRunCountsSetupBecauseItsTraceCannotNameIt(t *testing.T) { +func TestSummarizeRun_SeededRunLeavesTheSetupsLoginOutOfTheActionCount(t *testing.T) { seedDirectory := t.TempDir() writeRunDirectoryWithMeta(t, seedDirectory, "20260812-090000", trace.Meta{Seed: 11, Platform: "web", Arm: "seeded-baseline", Generator: "seeded"}, @@ -197,17 +204,73 @@ func TestSummarizeRun_SeededRunCountsSetupBecauseItsTraceCannotNameIt(t *testing setupStep(1), setupStep(2), setupStep(3), - actingStep(4), - actingStep(5), + seededStep(4), + seededStep(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) + if summary.Actions != 2 { + t.Errorf("actions: got %d, want 2 (three login steps were setup's), which is the same "+ + "rule the model arm is counted by", summary.Actions) + } + if summary.UnattributedActions != 0 { + t.Errorf("unattributed actions: got %d, want 0: every step named its source", + summary.UnattributedActions) + } +} + +func TestSummarizeRun_SeededRunWithoutSetupCountsEveryDispatchedStep(t *testing.T) { + seedDirectory := t.TempDir() + writeRunDirectoryWithMeta(t, seedDirectory, "20260812-090000", + trace.Meta{Seed: 11, Platform: "web", Arm: "seeded-baseline", Generator: "seeded"}, + []trace.Step{seededStep(1), seededStep(2), seededStep(3), seededStep(4)}) + + _, summary, err := summarizeRun(seedDirectory) + if err != nil { + t.Fatal(err) + } + if summary.Actions != 4 { + t.Errorf("actions: got %d, want 4", summary.Actions) + } +} + +// Traces recorded before actions named their source cannot be re-attributed +// after the fact, so each arm keeps the count it was already reported with: the +// seeded arm counts every dispatched step, the model arm counts only what the +// model stamped. UnattributedActions is how such a run says so rather than +// passing its old denominator off as a setup-excluding one. +func TestSummarizeRun_TraceWithoutSourcesKeepsTheCountItWasReportedWith(t *testing.T) { + for _, testCase := range []struct { + generator string + actions int + unattributedActions int + }{ + {generator: "seeded", actions: 5, unattributedActions: 5}, + {generator: "llm", actions: 0, unattributedActions: 5}, + } { + t.Run(testCase.generator, func(t *testing.T) { + seedDirectory := t.TempDir() + writeRunDirectoryWithMeta(t, seedDirectory, "20260812-090000", + trace.Meta{Seed: 11, Platform: "web", Generator: testCase.generator}, + []trace.Step{ + actingStep(1), actingStep(2), actingStep(3), actingStep(4), actingStep(5), + }) + + _, summary, err := summarizeRun(seedDirectory) + if err != nil { + t.Fatal(err) + } + if summary.Actions != testCase.actions { + t.Errorf("actions: got %d, want %d", summary.Actions, testCase.actions) + } + if summary.UnattributedActions != testCase.unattributedActions { + t.Errorf("unattributed actions: got %d, want %d", + summary.UnattributedActions, testCase.unattributedActions) + } + }) } } diff --git a/docs/manual/runs.md b/docs/manual/runs.md index 58104c4..d9f9694 100644 --- a/docs/manual/runs.md +++ b/docs/manual/runs.md @@ -59,6 +59,8 @@ Preconditions like login run through the spec's `setup` export (see the [case st | 30 min | ~15s | 0.8% | | 1 hour | ~15s | 0.4% | +Those steps are still actions the app received, so the trace names them: every action it records carries a `source` saying whether `setup`, the seeded picker or the model produced it. Anything measured per action counts the last two. + ## Session state Session tokens, keychain entries, shared preferences, and cookies survive the whole run. If the app logs the user out mid-run, the gating extractor flips, `setup` re-engages, and the run logs back in. No retry logic needed in the spec. diff --git a/docs/manual/spec-language.md b/docs/manual/spec-language.md index 54c2269..a57dfdd 100644 --- a/docs/manual/spec-language.md +++ b/docs/manual/spec-language.md @@ -343,6 +343,8 @@ Set `OPENROUTER_API_KEY` or `OPENAI_API_KEY` (OpenRouter wins if both are set). Each step it gets a screenshot plus a numbered list of the concrete actions your tree yields right now, each tagged with its weight, and picks one number. That list is the seeded picker's own candidate enumeration, so both modes explore the same action space and only the choice differs. `instructions` are appended to the prompt: say what the app is, not how to test it; the model works that part out. Everything else is unchanged. Setup actions still run first, typing still falls back to the edge-case corpus when the model supplies no text, and the trace records the reasoning, the chosen number, and `source: "llm"` so the replay UI can show why each pick happened. +Every dispatched action names its producer in the trace that way, whichever mode drove it: `"setup"` for a step the spec's `setup` drove, `"seeded"` for the seeded picker's own pick, `"llm"` for the model's. Only the last two explored anything, so a rate measured per action divides by those and leaves the login out on both modes. + One thing this mode refuses outright: a sampler drawn inside an `actions()` leaf. `from(...).generate()` draws from the seeded picker's stream, which the model never enters, so it would be offered the first item on every step while a seeded run reaches all of them. The run stops and names the leaf rather than degrade quietly. Return one action per item instead (`cards.map(card => Tap({ on: card }))`); a one-item list never draws and needs no change. The value generators draw from that same stream, so they refuse there too: an authored `InputText({ into: field, text: String(amounts.generate()) })` would type one fixed value on every model step while the seeded arm varies it, which is a different experiment rather than a different action space. Pass a fixed value instead. A generator whose span is a single value (`integers().between(7, 7)`, `strings().length(0, 0)`) never draws and is left alone, and `setup` is untouched: it runs through the picker with its seed under both generators, so a sampler there keeps drawing. diff --git a/internal/runner/llm_source.go b/internal/runner/llm_source.go index cb2953a..9e24809 100644 --- a/internal/runner/llm_source.go +++ b/internal/runner/llm_source.go @@ -65,14 +65,11 @@ type llmSource struct { // can stamp the trace. lastSource is "llm" only when the LLM (not setup) // chose the action; lastReasoning is the model's rationale. lastChoice is the // 1-based number it picked and lastChosenAction the description it echoed, so - // the trace shows what the model believed it was doing. lastFromSetup says - // the spec's setup produced the action, which is the app being put in - // position rather than the generator exploring it. + // the trace shows what the model believed it was doing. lastSource string lastReasoning string lastChoice int lastChosenAction string - lastFromSetup bool } // llmSelection is the outcome of one LLM selection call. @@ -99,14 +96,12 @@ func (s *llmSource) NextAction(ctx context.Context, stepIndex int) (verifier.Act s.lastReasoning = "" s.lastChoice = 0 s.lastChosenAction = "" - s.lastFromSetup = false s.history.completeLast(s.verifier.CurrentScreen()) // Setup precedence only: the LLM replaces the seeded action root, so we run // setup (e.g. login) first but never the weighted picker. action, err := s.verifier.SetupAction() if err == nil { - s.lastFromSetup = true s.record(stepIndex, trace.LLMCall{Outcome: trace.LLMOutcomeSetupAction}) s.history.add(describeAction(action, s.verifier.Tree())) return action, nil @@ -501,9 +496,10 @@ func (h *actionHistory) recent() []historyEntry { return h.entries } -// stampActionSource records the backend that chose an action on the trace. -// Only an LLM-selected action (not a setup action the JS path produced) carries -// source="llm" and the model's reasoning. +// stampActionSource names the model as the producer of an action it chose. The +// spec's two generators name themselves on the wire (runtime-entry.ts) and +// traceActionFor has already carried that over; a model pick is built here from +// the candidate list, so nothing on the wire could have named it. func stampActionSource(traceAction *trace.Action, source ActionSource) { if traceAction == nil { return @@ -518,14 +514,13 @@ func stampActionSource(traceAction *trace.Action, source ActionSource) { traceAction.LLMChosenAction = llm.lastChosenAction } -// generatorChoseAction reports whether the action the source just returned came -// from the generator rather than from the spec's setup driving the app into -// position. Only the model source can tell them apart: the seeded picker -// resolves setup precedence inside the one JS call it makes, so everything it -// returns counts as the generator's. -func generatorChoseAction(source ActionSource) bool { - llm, ok := source.(*llmSource) - return !ok || !llm.lastFromSetup +// generatorChoseAction reports whether this step drove the app on the action +// generator's behalf rather than on setup's, which is the exposure a per-action +// rate divides by. Both policies are read the same way, off the stamped trace +// action: an unstamped one is a source the runner cannot name, and counting it +// as setup would report a run that explored as having explored nothing. +func generatorChoseAction(traceAction *trace.Action) bool { + return traceAction != nil && traceAction.Source != trace.ActionSourceSetup } // screenshotDataURL downscales the PNG and encodes it as a data URL for the diff --git a/internal/runner/llm_source_test.go b/internal/runner/llm_source_test.go index 8165d78..f22121f 100644 --- a/internal/runner/llm_source_test.go +++ b/internal/runner/llm_source_test.go @@ -968,13 +968,13 @@ func TestRunner_SetupAndGeneratorBothDrivingIsAHealthyRun(t *testing.T) { } } for _, line := range lines[:2] { - if line.NextAction.Source != "" { - t.Errorf("step %d action source = %q, want none: setup chose it", + if line.NextAction.Source != trace.ActionSourceSetup { + t.Errorf("step %d action source = %q, want setup: setup chose it", line.Step, line.NextAction.Source) } } for _, line := range lines[2:] { - if line.NextAction.Source != "llm" { + if line.NextAction.Source != trace.ActionSourceModel { t.Errorf("step %d action source = %q, want llm", line.Step, line.NextAction.Source) } } diff --git a/internal/runner/runner.go b/internal/runner/runner.go index 33c727c..dd9ed16 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -82,9 +82,8 @@ type Summary struct { // GeneratorActions counts the dispatched actions the generator chose. The // spec's setup drives the app into its starting position before the // generator is consulted, so a run at zero here explored nothing however - // many actions its login fired. Only the model generator separates the two: - // the seeded picker resolves setup precedence inside the one JS call it - // makes, so everything it returns counts as the generator's. + // many actions its login fired. Both generators separate the two the same + // way, by the producer each action names on the trace. GeneratorActions int } @@ -506,7 +505,7 @@ func Run(ctx context.Context, options Options) (Summary, error) { } if nextErr == nil && !applySkipped { summary.DispatchedActions++ - if generatorChoseAction(actionSource) { + if generatorChoseAction(traceAction) { summary.GeneratorActions++ } } @@ -1580,7 +1579,12 @@ func driverIsAndroid(ctx context.Context, options Options, logger *slog.Logger) } func traceActionFor(action verifier.Action, tree *hierarchy.Tree) *trace.Action { - traceAction := &trace.Action{Kind: string(action.Kind), X: action.X, Y: action.Y} + traceAction := &trace.Action{ + Kind: string(action.Kind), + X: action.X, + Y: action.Y, + Source: action.Source, + } switch action.Kind { case verifier.ActionKindTap, verifier.ActionKindDoubleTap, verifier.ActionKindLongPress: traceAction.Selector = action.On diff --git a/internal/runner/runner_test.go b/internal/runner/runner_test.go index 5d6e75c..12cb2d4 100644 --- a/internal/runner/runner_test.go +++ b/internal/runner/runner_test.go @@ -223,6 +223,62 @@ func TestRunner_SeededRunRecordsNoModelCalls(t *testing.T) { } } +// seededLoginSetupSpec drives the first two steps from setup, the way a +// login-fronted spec does, and leaves the rest to the seeded action root. +const seededLoginSetupSpec = ` +import { always, actions, taps, typing, weighted, Tap } from "@sanderling/spec"; +globalThis.properties = { ok: always(() => true) }; +let setupTapsLeft = 2; +globalThis.setup = actions(() => (setupTapsLeft-- > 0 ? [Tap({ on: "id:Submit" })] : [])); +globalThis.actions = weighted([1, taps], [1, typing]); +` + +// TestRunner_SeededSetupActionsAreNotTheGeneratorDrivingTheApp: a seeded run's +// login steps used to be indistinguishable from its exploration, so the arm +// divided its defect rate by every action it dispatched while the model arm +// divided by the ones its policy chose. The two rates were then compared. +func TestRunner_SeededSetupActionsAreNotTheGeneratorDrivingTheApp(t *testing.T) { + state := newHarnessWithSpec(t, seededLoginSetupSpec) + state.mock.HierarchyJSON = llmTreeJSON + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + summary, err := Run(ctx, Options{ + Duration: 30 * time.Second, + IdleTimeout: 20 * time.Millisecond, + MaxSteps: 4, + Driver: state.mock, + Verifier: state.verifier, + TraceWriter: state.writer, + Logger: slog.New(slog.NewTextHandler(io.Discard, nil)), + }) + if err != nil { + t.Fatalf("Run: %v", err) + } + if summary.DispatchedActions != 4 { + t.Errorf("DispatchedActions = %d, want 4: every step drove the app", + summary.DispatchedActions) + } + if summary.GeneratorActions != 2 { + t.Errorf("GeneratorActions = %d, want 2: the picker drove the two steps setup left it", + summary.GeneratorActions) + } + lines := readTraceLines(t, state.writer.Directory()) + if len(lines) != 4 { + t.Fatalf("wrote %d trace lines, want 4", len(lines)) + } + for _, line := range lines[:2] { + if line.NextAction == nil || line.NextAction.Source != trace.ActionSourceSetup { + t.Errorf("step %d action = %+v, want one named setup", line.Step, line.NextAction) + } + } + for _, line := range lines[2:] { + if line.NextAction == nil || line.NextAction.Source != trace.ActionSourceSeeded { + t.Errorf("step %d action = %+v, want one named seeded", line.Step, line.NextAction) + } + } +} + func TestRunner_MaxStepsStopsAfterExactlyNSteps(t *testing.T) { state := newHarness(t) diff --git a/internal/runner/testdata/trace.jsonl b/internal/runner/testdata/trace.jsonl index 7ed3324..58bbfe1 100644 --- a/internal/runner/testdata/trace.jsonl +++ b/internal/runner/testdata/trace.jsonl @@ -1,4 +1,4 @@ -{"extractor_changes":{"extractor_0":{"prev":null,"curr":0}},"hierarchy":{"elements":[{"resourceId":"HomeScreen","bounds":{"left":0,"top":0,"right":0,"bottom":0},"attrs":{"editable":"false","resource-id":"HomeScreen"}},{"resourceId":"next","clickable":true,"enabled":true,"bounds":{"left":40,"top":80,"right":240,"bottom":160},"attrs":{"bounds":"[40,80,240,160]","clickable":"true","editable":"false","enabled":"true","resource-id":"next"}}],"depths":[0,1]},"next_action":{"kind":"Tap","selector":"id:next","resolved_bounds":{"x":40,"y":80,"width":200,"height":80},"tap_point":{"x":140,"y":120}},"residuals":{"balanceNonNegative":{"op":"true"}},"step":1,"timestamp":"0001-01-01T00:00:00Z","trace_version":1} -{"hierarchy":{"elements":[{"resourceId":"HomeScreen","bounds":{"left":0,"top":0,"right":0,"bottom":0},"attrs":{"editable":"false","resource-id":"HomeScreen"}},{"resourceId":"next","clickable":true,"enabled":true,"bounds":{"left":40,"top":80,"right":240,"bottom":160},"attrs":{"bounds":"[40,80,240,160]","clickable":"true","editable":"false","enabled":"true","resource-id":"next"}}],"depths":[0,1]},"next_action":{"kind":"Tap","selector":"id:next","resolved_bounds":{"x":40,"y":80,"width":200,"height":80},"tap_point":{"x":140,"y":120}},"residuals":{"balanceNonNegative":{"op":"true"}},"step":2,"timestamp":"0001-01-01T00:00:00Z","trace_version":1} -{"hierarchy":{"elements":[{"resourceId":"HomeScreen","bounds":{"left":0,"top":0,"right":0,"bottom":0},"attrs":{"editable":"false","resource-id":"HomeScreen"}},{"resourceId":"next","clickable":true,"enabled":true,"bounds":{"left":40,"top":80,"right":240,"bottom":160},"attrs":{"bounds":"[40,80,240,160]","clickable":"true","editable":"false","enabled":"true","resource-id":"next"}}],"depths":[0,1]},"next_action":{"kind":"Tap","selector":"id:next","resolved_bounds":{"x":40,"y":80,"width":200,"height":80},"tap_point":{"x":140,"y":120}},"residuals":{"balanceNonNegative":{"op":"true"}},"step":3,"timestamp":"0001-01-01T00:00:00Z","trace_version":1} -{"hierarchy":{"elements":[{"resourceId":"HomeScreen","bounds":{"left":0,"top":0,"right":0,"bottom":0},"attrs":{"editable":"false","resource-id":"HomeScreen"}},{"resourceId":"next","clickable":true,"enabled":true,"bounds":{"left":40,"top":80,"right":240,"bottom":160},"attrs":{"bounds":"[40,80,240,160]","clickable":"true","editable":"false","enabled":"true","resource-id":"next"}}],"depths":[0,1]},"next_action":{"kind":"Tap","selector":"id:next","resolved_bounds":{"x":40,"y":80,"width":200,"height":80},"tap_point":{"x":140,"y":120}},"residuals":{"balanceNonNegative":{"op":"true"}},"step":4,"timestamp":"0001-01-01T00:00:00Z","trace_version":1} +{"extractor_changes":{"extractor_0":{"prev":null,"curr":0}},"hierarchy":{"elements":[{"resourceId":"HomeScreen","bounds":{"left":0,"top":0,"right":0,"bottom":0},"attrs":{"editable":"false","resource-id":"HomeScreen"}},{"resourceId":"next","clickable":true,"enabled":true,"bounds":{"left":40,"top":80,"right":240,"bottom":160},"attrs":{"bounds":"[40,80,240,160]","clickable":"true","editable":"false","enabled":"true","resource-id":"next"}}],"depths":[0,1]},"next_action":{"kind":"Tap","selector":"id:next","resolved_bounds":{"x":40,"y":80,"width":200,"height":80},"tap_point":{"x":140,"y":120},"source":"seeded"},"residuals":{"balanceNonNegative":{"op":"true"}},"step":1,"timestamp":"0001-01-01T00:00:00Z","trace_version":1} +{"hierarchy":{"elements":[{"resourceId":"HomeScreen","bounds":{"left":0,"top":0,"right":0,"bottom":0},"attrs":{"editable":"false","resource-id":"HomeScreen"}},{"resourceId":"next","clickable":true,"enabled":true,"bounds":{"left":40,"top":80,"right":240,"bottom":160},"attrs":{"bounds":"[40,80,240,160]","clickable":"true","editable":"false","enabled":"true","resource-id":"next"}}],"depths":[0,1]},"next_action":{"kind":"Tap","selector":"id:next","resolved_bounds":{"x":40,"y":80,"width":200,"height":80},"tap_point":{"x":140,"y":120},"source":"seeded"},"residuals":{"balanceNonNegative":{"op":"true"}},"step":2,"timestamp":"0001-01-01T00:00:00Z","trace_version":1} +{"hierarchy":{"elements":[{"resourceId":"HomeScreen","bounds":{"left":0,"top":0,"right":0,"bottom":0},"attrs":{"editable":"false","resource-id":"HomeScreen"}},{"resourceId":"next","clickable":true,"enabled":true,"bounds":{"left":40,"top":80,"right":240,"bottom":160},"attrs":{"bounds":"[40,80,240,160]","clickable":"true","editable":"false","enabled":"true","resource-id":"next"}}],"depths":[0,1]},"next_action":{"kind":"Tap","selector":"id:next","resolved_bounds":{"x":40,"y":80,"width":200,"height":80},"tap_point":{"x":140,"y":120},"source":"seeded"},"residuals":{"balanceNonNegative":{"op":"true"}},"step":3,"timestamp":"0001-01-01T00:00:00Z","trace_version":1} +{"hierarchy":{"elements":[{"resourceId":"HomeScreen","bounds":{"left":0,"top":0,"right":0,"bottom":0},"attrs":{"editable":"false","resource-id":"HomeScreen"}},{"resourceId":"next","clickable":true,"enabled":true,"bounds":{"left":40,"top":80,"right":240,"bottom":160},"attrs":{"bounds":"[40,80,240,160]","clickable":"true","editable":"false","enabled":"true","resource-id":"next"}}],"depths":[0,1]},"next_action":{"kind":"Tap","selector":"id:next","resolved_bounds":{"x":40,"y":80,"width":200,"height":80},"tap_point":{"x":140,"y":120},"source":"seeded"},"residuals":{"balanceNonNegative":{"op":"true"}},"step":4,"timestamp":"0001-01-01T00:00:00Z","trace_version":1} diff --git a/internal/trace/writer.go b/internal/trace/writer.go index 066920c..a8c4ee6 100644 --- a/internal/trace/writer.go +++ b/internal/trace/writer.go @@ -109,6 +109,16 @@ type Metrics struct { TotalMemoryBytes int64 `json:"total_memory_bytes,omitempty"` } +// The three producers an action can come from. The spec's setup drives the app +// into position and explores nothing, so a per-action rate divides by the other +// two; an action recorded before this distinction existed carries none of them +// and cannot be attributed after the fact. +const ( + ActionSourceSetup = "setup" + ActionSourceSeeded = "seeded" + ActionSourceModel = "llm" +) + type Action struct { Kind string `json:"kind"` X int `json:"x,omitempty"` @@ -123,8 +133,8 @@ type Action struct { Selector string `json:"selector,omitempty"` ResolvedBounds *BoundsRecord `json:"resolved_bounds,omitempty"` TapPoint *PointRecord `json:"tap_point,omitempty"` - // Source names the backend that chose this action: "llm" when the LLM - // action backend selected it, empty for the seeded picker. LLMReasoning is + // Source names which of the three producers chose this action, and is empty + // only on a trace recorded before actions named themselves. LLMReasoning is // the model's short rationale, shown by the replay UI to explain the pick. Source string `json:"source,omitempty"` LLMReasoning string `json:"llm_reasoning,omitempty"` diff --git a/internal/verifier/marshal.go b/internal/verifier/marshal.go index 6b488de..6d82310 100644 --- a/internal/verifier/marshal.go +++ b/internal/verifier/marshal.go @@ -611,6 +611,10 @@ type wireAction struct { ToX int `json:"toX"` ToY int `json:"toY"` DurationMillis int `json:"durationMillis"` + // Source names the generator that produced the action: the spec's setup or + // the action root. Empty from the candidate enumeration, which serializes + // actions nothing has chosen yet. + Source string `json:"source"` } // DecodeAction turns one serialized action (the flat camelCase wire contract) @@ -623,6 +627,15 @@ func DecodeAction(raw json.RawMessage) (Action, error) { if err := json.Unmarshal(raw, &wire); err != nil { return Action{}, fmt.Errorf("decode action: %w", err) } + action, err := actionFromWire(wire) + if err != nil { + return Action{}, err + } + action.Source = wire.Source + return action, nil +} + +func actionFromWire(wire wireAction) (Action, error) { switch wire.Kind { case "Tap": return Action{Kind: ActionKindTap, On: wire.Selector, X: wire.X, Y: wire.Y}, nil diff --git a/internal/verifier/policy_parity_test.go b/internal/verifier/policy_parity_test.go index 3472d3e..2eada0e 100644 --- a/internal/verifier/policy_parity_test.go +++ b/internal/verifier/policy_parity_test.go @@ -184,12 +184,15 @@ func modelOfferedActions(t *testing.T, verb string) map[string]Action { // actionIdentity keys an action by everything except the values the policy owns // rather than the candidate set: the typed text, which the seeded arm draws from -// the edge-case corpus and the model writes itself, and a swipe's drag distance, -// which the seeded arm draws and the enumeration lists at a nominal length. +// the edge-case corpus and the model writes itself, a swipe's drag distance, +// which the seeded arm draws and the enumeration lists at a nominal length, and +// the source, which names who produced an action rather than what it does (the +// enumeration names nobody, because the model has not chosen yet). // Comparing those would compare policies instead of action spaces. A swipe's // direction is NOT policy-owned, so it survives as the sign of the drag. func actionIdentity(action Action) string { action.Text = "" + action.Source = "" if action.Kind == ActionKindSwipe { action.ToX = sign(action.ToX - action.FromX) action.ToY = sign(action.ToY - action.FromY) diff --git a/internal/verifier/setup_action_test.go b/internal/verifier/setup_action_test.go index b7798c7..7d41b08 100644 --- a/internal/verifier/setup_action_test.go +++ b/internal/verifier/setup_action_test.go @@ -96,6 +96,58 @@ export const actionsRoot = taps; } } +// TestNextActionNamesTheGeneratorThatProducedIt is the native half of the +// cross-host marker contract: for the same spec shape, a setup-driven step and +// an action-root step must name different producers. The web half asserts the +// same two names in pkg/spec/test/web-runtime.test.ts, so a match on both sides +// proves the engines agree without either invoking the other. A per-action rate +// divides by the root's steps only, and an unnamed producer inflates it by +// however many steps the login consumed. +func TestNextActionNamesTheGeneratorThatProducedIt(t *testing.T) { + spec := ` +import { Tap, actions, extract, taps } from "@sanderling/spec"; +const signIn = extract("signIn", state => state.ax.find("id:SignIn")); +export const setup = actions(() => (signIn.current ? [Tap({ on: "id:SignIn" })] : [])); +export const actionsRoot = taps; +` + v := loadBundled(t, spec, enumTreeJSON) + action, err := v.NextAction() + if err != nil { + t.Fatalf("NextAction: %v", err) + } + if action.On != "id:SignIn" || action.Source != "setup" { + t.Errorf("NextAction = %+v, want the Tap on id:SignIn named setup", action) + } + + pushTree(t, v, policyTreeJSON) + action, err = v.NextAction() + if err != nil { + t.Fatalf("NextAction after setup went quiet: %v", err) + } + if action.Source != "seeded" { + t.Errorf("NextAction = %+v, want the action root's tap named seeded", action) + } +} + +// TestSetupActionNamesSetup: the model policy reaches setup through its own +// entry, which must name the producer the same way the seeded entry does, or +// the two arms' per-action rates divide by different things. +func TestSetupActionNamesSetup(t *testing.T) { + spec := ` +import { Tap, actions, taps } from "@sanderling/spec"; +export const setup = actions(() => [Tap({ on: "id:SignIn" })]); +export const actionsRoot = taps; +` + v := loadBundled(t, spec, enumTreeJSON) + action, err := v.SetupAction() + if err != nil { + t.Fatalf("SetupAction: %v", err) + } + if action.Source != "setup" { + t.Errorf("SetupAction = %+v, want it named setup", action) + } +} + // TestSetupGeneratorDrawsUnderTheModelPolicy: setup walks through the picker // with its rng under both policies, so a generator there is not the divergence // the enumeration refuses and must keep drawing. Enumerating before every setup diff --git a/internal/verifier/types.go b/internal/verifier/types.go index 09d2c5c..3c29cda 100644 --- a/internal/verifier/types.go +++ b/internal/verifier/types.go @@ -38,6 +38,10 @@ type Action struct { // when the apply call failed and nothing can say whether the action // reached the app. The spec is told which of the two it is. Applied bool + // Source names the generator that produced this action, "setup" or + // "seeded", as the runtime entry tagged it. Empty on an action the runner + // built itself (the model policy's pick), which the runner names instead. + Source string // Relaunched, like Applied, is meaningful only on state.lastAction: the // runner brought the app back to the foreground after this action, so the // two readings the spec compares straddle a restart. The action still diff --git a/pkg/spec/src/runtime-entry.ts b/pkg/spec/src/runtime-entry.ts index 6055bf5..2436982 100644 --- a/pkg/spec/src/runtime-entry.ts +++ b/pkg/spec/src/runtime-entry.ts @@ -18,7 +18,9 @@ import type { Point } from "./types.ts"; // (ONE decoder on each side). Builtin targets are already resolved to a Point, // and serializeAction collapses author targets that carry {x, y}; a target that // does not resolve to coordinates drops the action (returns null). -export type SerializedAction = +export type SerializedAction = SerializedActionShape & { source?: ActionSource }; + +type SerializedActionShape = | { kind: "Tap" | "DoubleTap" | "LongPress"; x: number; y: number; selector?: string } | { kind: "InputText"; x: number; y: number; text: string; selector?: string } | { kind: "Swipe"; fromX: number; fromY: number; toX: number; toY: number; durationMillis: number } @@ -35,6 +37,12 @@ export type SerializedAction = | { kind: "PressKey"; key: string } | { kind: "Wait"; durationMillis: number }; +// ActionSource names which of the spec's two generators produced an action. +// Both are dispatched the same way, but only the action root explores: setup +// drives the app into its starting position, so a per-action rate that counts +// its login steps measures a policy's exposure as larger than it was. +export type ActionSource = "setup" | "seeded"; + const DEFAULT_SWIPE_DURATION = 250; // pointOf resolves a target to {x, y, selector?}. Builtins and resolved ax @@ -59,7 +67,19 @@ function pointOf(target: unknown): (Point & { selector?: string }) | undefined { return undefined; } -export function serializeAction(action: ActionDescriptor | null): SerializedAction | null { +// serializeAction emits the wire shape, tagged with the generator that produced +// the action when the caller knows it. The candidate enumeration the model +// policy reads passes no source: nothing has chosen those yet. +export function serializeAction( + action: ActionDescriptor | null, + source?: ActionSource, +): SerializedAction | null { + const wire = serializeWire(action); + if (!wire || !source) return wire; + return { ...wire, source }; +} + +function serializeWire(action: ActionDescriptor | null): SerializedAction | null { if (!action) return null; switch (action.kind) { case "Tap": @@ -184,7 +204,7 @@ export function installRuntime( resolveRoot(); const setup = resolveSetup(); if (!setup) return null; - return serializeAction(walk(setup, rng, host)); + return serializeAction(walk(setup, rng, host), "setup"); }); defineLockedGlobal("__sanderlingNextAction__", () => { // resolveRoot runs first: on web it also resets the per-tick candidate @@ -193,10 +213,10 @@ export function installRuntime( const setup = resolveSetup(); if (setup) { const setupAction = walk(setup, rng, host); - if (setupAction) return serializeAction(setupAction); + if (setupAction) return serializeAction(setupAction, "setup"); } if (!current) return null; - return serializeAction(nextAction(current, rng, host)); + return serializeAction(nextAction(current, rng, host), "seeded"); }); } diff --git a/pkg/spec/test/fixtures/parity-golden.json b/pkg/spec/test/fixtures/parity-golden.json index b9865d4..4fc17ca 100644 --- a/pkg/spec/test/fixtures/parity-golden.json +++ b/pkg/spec/test/fixtures/parity-golden.json @@ -3,125 +3,145 @@ "kind": "Tap", "x": 250, "y": 260, - "selector": "id:gamma" + "selector": "id:gamma", + "source": "seeded" }, { "kind": "Tap", "x": 150, "y": 160, - "selector": "id:beta" + "selector": "id:beta", + "source": "seeded" }, { "kind": "Tap", "x": 50, "y": 60, - "selector": "id:alpha" + "selector": "id:alpha", + "source": "seeded" }, { "kind": "InputText", "x": 150, "y": 160, "text": "a", - "selector": "id:beta" + "selector": "id:beta", + "source": "seeded" }, { "kind": "InputText", "x": 50, "y": 60, "text": "\t\n", - "selector": "id:alpha" + "selector": "id:alpha", + "source": "seeded" }, { "kind": "Tap", "x": 250, "y": 260, - "selector": "id:gamma" + "selector": "id:gamma", + "source": "seeded" }, { "kind": "Tap", "x": 150, "y": 160, - "selector": "id:beta" + "selector": "id:beta", + "source": "seeded" }, { "kind": "Tap", "x": 250, "y": 260, - "selector": "id:gamma" + "selector": "id:gamma", + "source": "seeded" }, { "kind": "Tap", "x": 150, "y": 160, - "selector": "id:beta" + "selector": "id:beta", + "source": "seeded" }, { "kind": "Tap", "x": 250, "y": 260, - "selector": "id:gamma" + "selector": "id:gamma", + "source": "seeded" }, { "kind": "Tap", "x": 250, "y": 260, - "selector": "id:gamma" + "selector": "id:gamma", + "source": "seeded" }, { "kind": "Tap", "x": 150, "y": 160, - "selector": "id:beta" + "selector": "id:beta", + "source": "seeded" }, { "kind": "Tap", "x": 250, "y": 260, - "selector": "id:gamma" + "selector": "id:gamma", + "source": "seeded" }, { "kind": "Tap", "x": 50, "y": 60, - "selector": "id:alpha" + "selector": "id:alpha", + "source": "seeded" }, { "kind": "InputText", "x": 150, "y": 160, "text": "999999999999999999999", - "selector": "id:beta" + "selector": "id:beta", + "source": "seeded" }, { "kind": "InputText", "x": 150, "y": 160, "text": "-1", - "selector": "id:beta" + "selector": "id:beta", + "source": "seeded" }, { "kind": "InputText", "x": 250, "y": 260, "text": "-1", - "selector": "id:gamma" + "selector": "id:gamma", + "source": "seeded" }, { "kind": "Tap", "x": 50, "y": 60, - "selector": "id:alpha" + "selector": "id:alpha", + "source": "seeded" }, { "kind": "Tap", "x": 250, "y": 260, - "selector": "id:gamma" + "selector": "id:gamma", + "source": "seeded" }, { "kind": "Tap", "x": 50, "y": 60, - "selector": "id:alpha" + "selector": "id:alpha", + "source": "seeded" } ] diff --git a/pkg/spec/test/parity-harness.ts b/pkg/spec/test/parity-harness.ts index bda4e9b..142662a 100644 --- a/pkg/spec/test/parity-harness.ts +++ b/pkg/spec/test/parity-harness.ts @@ -45,12 +45,14 @@ const HOST: Host = { // runParity emits the parity scenario's action stream from a fresh Pcg. It // drives pick.ts directly (not installRuntime, whose globals are locked once) -// so the caller can run it repeatedly to assert determinism. +// so the caller can run it repeatedly to assert determinism. The scenario has +// no setup, so it stands in for the entry's action-root branch and tags what it +// emits the way that branch does. export function runParity(): (SerializedAction | null)[] { const rng = new Pcg(PARITY_SEED_HI, 0n); const stream: (SerializedAction | null)[] = []; for (let i = 0; i < PARITY_STEPS; i++) { - stream.push(serializeAction(nextAction(PARITY_ROOT, rng, HOST))); + stream.push(serializeAction(nextAction(PARITY_ROOT, rng, HOST), "seeded")); } return stream; } diff --git a/pkg/spec/test/web-runtime.test.ts b/pkg/spec/test/web-runtime.test.ts index 4cdf781..43464bf 100644 --- a/pkg/spec/test/web-runtime.test.ts +++ b/pkg/spec/test/web-runtime.test.ts @@ -1129,3 +1129,31 @@ test("a nested undefined leaves the page as a dropped key, a nested null does no }); assert.equal(wire, `{"0":{"value":{"empty":null,"present":1}}}`); }); + +// The web half of the cross-host marker contract: for the same spec shape, the +// entry must name setup's action and the action root's differently, and by the +// same two names the goja engine uses (TestNextActionNamesTheGeneratorThatProducedIt +// in internal/verifier/setup_action_test.go asserts them there). A per-action +// rate divides by the root's steps only, so an action that names no producer +// puts a login's taps in the denominator of the policy's exploration. +const { Tap, actions, taps } = await import("../src/actions.ts"); + +test("the next-action entry names the generator each action came from", () => { + const button = fakeElement({ + tag: "button", x: 0, y: 0, width: 40, height: 20, id: "SignIn", clickable: true, + }); + const g = globalThis as { actions?: unknown; setup?: unknown; __sanderlingNextAction__?: unknown }; + const nextAction = g.__sanderlingNextAction__ as () => { source?: string } | null; + withFakeDocument([button], () => { + try { + g.actions = taps; + g.setup = actions(() => [Tap({ on: "id:SignIn" })]); + assert.equal(nextAction()?.source, "setup"); + g.setup = undefined; + assert.equal(nextAction()?.source, "seeded"); + } finally { + g.actions = undefined; + g.setup = undefined; + } + }); +});