diff --git a/internal/runner/llm_source.go b/internal/runner/llm_source.go index 6f9f306..69df7cf 100644 --- a/internal/runner/llm_source.go +++ b/internal/runner/llm_source.go @@ -50,8 +50,12 @@ type llmSource struct { // instructions is optional spec-level guidance appended to the system prompt // to steer the model's bug-hunting (empty when unset). instructions string - logger *slog.Logger - history *actionHistory + // labelSource is the channel each candidate's target is named by in the + // numbered list. It lives here rather than on the verifier because only this + // policy reads labels at all. + labelSource string + logger *slog.Logger + history *actionHistory // recorder persists one record per step: what was sent, what came back, and // how the step ended. Nil only in unit tests that never select. recorder llmCallRecorder @@ -127,7 +131,7 @@ func (s *llmSource) NextAction(ctx context.Context, stepIndex int) (verifier.Act // usable. func (s *llmSource) selectViaLLM(ctx context.Context) (llmSelection, trace.LLMCall) { call := trace.LLMCall{Timestamp: time.Now(), Model: s.model} - candidates := s.verifier.Candidates() + candidates := s.verifier.Candidates(s.labelSource) if len(candidates) == 0 { call.Outcome = trace.LLMOutcomeNoCandidates return llmSelection{}, call diff --git a/internal/runner/llm_source_test.go b/internal/runner/llm_source_test.go index c69b942..e02b11e 100644 --- a/internal/runner/llm_source_test.go +++ b/internal/runner/llm_source_test.go @@ -275,6 +275,20 @@ func pushLLMSnapshot(t *testing.T, v *verifier.Verifier) { pushLLMSnapshotAtStep(t, v, 0) } +func pushSnapshotTree(t *testing.T, v *verifier.Verifier, treeJSON string) { + t.Helper() + tree, err := hierarchy.Parse(treeJSON) + if err != nil { + t.Fatal(err) + } + if err := v.PushSnapshot(verifier.SnapshotInput{ + Tree: tree, + ScreenshotPNG: tinyPNG(t), + }); err != nil { + t.Fatalf("PushSnapshot: %v", err) + } +} + func pushLLMSnapshotAtStep(t *testing.T, v *verifier.Verifier, stepIndex int) { t.Helper() tree, err := hierarchy.Parse(llmTreeJSON) @@ -351,6 +365,93 @@ func TestPickSourcesSeededByDefault(t *testing.T) { } } +// TestPickSourcesGivesTheLabelSourceToTheModelPickerOnly pins the asymmetry the +// labelling factorial depends on: the label channel reaches the model picker, +// and the seeded picker is handed a source that has nowhere to put one. +func TestPickSourcesGivesTheLabelSourceToTheModelPickerOnly(t *testing.T) { + fake := newFakeOpenRouter(t) + _, verifierInstance := newLLMSource(t, fake) + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + + action, _, err := pickSources(Options{ + Verifier: verifierInstance, + Generator: "llm", + LabelSource: verifier.LabelSourceResourceID, + Logger: logger, + }) + if err != nil { + t.Fatalf("pickSources: %v", err) + } + model, ok := action.(*llmSource) + if !ok { + t.Fatalf("action source = %T, want *llmSource", action) + } + if model.labelSource != verifier.LabelSourceResourceID { + t.Errorf("labelSource = %q, want %q", model.labelSource, verifier.LabelSourceResourceID) + } + + seeded, _, err := pickSources(Options{ + Verifier: verifierInstance, + Generator: "seeded", + LabelSource: verifier.LabelSourceResourceID, + Logger: logger, + }) + if err != nil { + t.Fatalf("pickSources: %v", err) + } + if _, ok := seeded.(gojaSource); !ok { + t.Errorf("action source = %T, want gojaSource, which carries no label channel", seeded) + } +} + +// labelSplitTreeJSON names one control two ways, so a record can say which +// channel the model was reading. +const labelSplitTreeJSON = `{ + "attributes": {"bounds": "[0,0,400,800]"}, + "children": [ + {"attributes": {"resource-id": "add_credit_button", "text": "Add credit", "bounds": "[0,0,400,100]"}, "clickable": true, "enabled": true, "children": []} + ] +}` + +// TestLLMSourceRecordsTheLabelsTheModelSaw closes the loop from the flag to the +// artifact: llm-calls.jsonl carries the candidate list as rendered, so a +// directory of runs can be checked against the cell it claims rather than +// trusted. +func TestLLMSourceRecordsTheLabelsTheModelSaw(t *testing.T) { + for _, want := range []struct{ labelSource, label string }{ + {verifier.LabelSourceVisibleText, "Add credit"}, + {verifier.LabelSourceResourceID, "add_credit_button"}, + } { + t.Run(want.labelSource, func(t *testing.T) { + fake := newFakeOpenRouter(t) + source, verifierInstance := newLLMSource(t, fake) + source.labelSource = want.labelSource + pushSnapshotTree(t, verifierInstance, labelSplitTreeJSON) + + tap := candidateByKind(t, verifierInstance.Candidates(want.labelSource), verifier.ActionKindTap) + fake.choice = tap.Index + fake.chosenAction = tap.Description + if _, err := source.NextAction(context.Background(), 0); err != nil { + t.Fatalf("NextAction: %v", err) + } + + call := lastCall(t, source) + if call.Outcome != trace.LLMOutcomeSelected { + t.Fatalf("outcome = %q, want selected", call.Outcome) + } + if len(call.Candidates) == 0 { + t.Fatal("no candidates recorded") + } + if call.Candidates[0].Label != want.label { + t.Errorf("recorded label = %q, want %q", call.Candidates[0].Label, want.label) + } + if !strings.Contains(call.UserPrompt, want.label) { + t.Errorf("prompt does not carry %q:\n%s", want.label, call.UserPrompt) + } + }) + } +} + // seededFixtureSpec declares no generator = llm(...), so --generator llm has // nothing to build a picker from. const seededFixtureSpec = ` @@ -449,7 +550,7 @@ func TestLLMSourceDrivesExecutedActions(t *testing.T) { fake := newFakeOpenRouter(t) source, verifierInstance := newLLMSource(t, fake) pushLLMSnapshot(t, verifierInstance) - candidates := verifierInstance.Candidates() + candidates := verifierInstance.Candidates(verifier.LabelSourceVisibleText) // Step 1: the model picks the Tap on Submit by its number, echoing its // description. @@ -528,7 +629,7 @@ func TestLLMSourceAcceptsEchoWithWeightSuffix(t *testing.T) { fake := newFakeOpenRouter(t) source, verifierInstance := newLLMSource(t, fake) pushLLMSnapshot(t, verifierInstance) - candidates := verifierInstance.Candidates() + candidates := verifierInstance.Candidates(verifier.LabelSourceVisibleText) tap := candidateByKind(t, candidates, verifier.ActionKindTap) fake.choice = tap.Index @@ -563,7 +664,7 @@ func TestLLMSourceStrictSkipsOnEchoMismatch(t *testing.T) { fake := newFakeOpenRouter(t) source, verifierInstance := newLLMSource(t, fake) pushLLMSnapshot(t, verifierInstance) - candidates := verifierInstance.Candidates() + candidates := verifierInstance.Candidates(verifier.LabelSourceVisibleText) // A valid number, but the echoed action disagrees with that numbered entry: // the model reasoned about one control and picked another's number. @@ -617,7 +718,7 @@ func TestLLMCallRecordSeparatesGuardSkipFromDecline(t *testing.T) { fake := newFakeOpenRouter(t) source, verifierInstance := newLLMSource(t, fake) pushLLMSnapshot(t, verifierInstance) - tap := candidateByKind(t, verifierInstance.Candidates(), verifier.ActionKindTap) + tap := candidateByKind(t, verifierInstance.Candidates(verifier.LabelSourceVisibleText), verifier.ActionKindTap) fake.choice = tap.Index fake.chosenAction = `Tap "Something Else"` if _, err := source.NextAction(context.Background(), stepIndex); !errors.Is(err, verifier.ErrNoAction) { @@ -638,7 +739,7 @@ func TestLLMCallRecordSeparatesGuardSkipFromDecline(t *testing.T) { fake := newFakeOpenRouter(t) source, verifierInstance := newLLMSource(t, fake) pushLLMSnapshot(t, verifierInstance) - tap := candidateByKind(t, verifierInstance.Candidates(), verifier.ActionKindTap) + tap := candidateByKind(t, verifierInstance.Candidates(verifier.LabelSourceVisibleText), verifier.ActionKindTap) fake.choice = tap.Index fake.chosenAction = tap.Description if _, err := source.NextAction(context.Background(), stepIndex); err != nil { @@ -686,7 +787,7 @@ func TestLLMCallRecordsCandidateListAsShown(t *testing.T) { source, verifierInstance := newLLMSource(t, fake) source.instructions = "hunt for double submits" pushLLMSnapshot(t, verifierInstance) - tap := candidateByKind(t, verifierInstance.Candidates(), verifier.ActionKindTap) + tap := candidateByKind(t, verifierInstance.Candidates(verifier.LabelSourceVisibleText), verifier.ActionKindTap) fake.choice = tap.Index fake.chosenAction = tap.Description if _, err := source.NextAction(context.Background(), 1); err != nil { @@ -774,7 +875,7 @@ func TestLLMCallFileRecordsUsageLatencyAndScreenshot(t *testing.T) { source.recorder = writer pushLLMSnapshotAtStep(t, verifierInstance, 4) - tap := candidateByKind(t, verifierInstance.Candidates(), verifier.ActionKindTap) + tap := candidateByKind(t, verifierInstance.Candidates(verifier.LabelSourceVisibleText), verifier.ActionKindTap) fake.choice = tap.Index fake.chosenAction = tap.Description if _, err := source.NextAction(context.Background(), 4); err != nil { @@ -814,7 +915,7 @@ func TestLLMCallScreenshotNamesObservedStep(t *testing.T) { fake := newFakeOpenRouter(t) source, verifierInstance := newLLMSource(t, fake) pushLLMSnapshotAtStep(t, verifierInstance, 4) - tap := candidateByKind(t, verifierInstance.Candidates(), verifier.ActionKindTap) + tap := candidateByKind(t, verifierInstance.Candidates(verifier.LabelSourceVisibleText), verifier.ActionKindTap) fake.choice = tap.Index fake.chosenAction = tap.Description if _, err := source.NextAction(context.Background(), 6); err != nil { diff --git a/internal/runner/runner.go b/internal/runner/runner.go index 529db61..5e322d8 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -40,6 +40,11 @@ type Options struct { // spec's generator = llm({...}) config; anything else (the default) uses the // seeded weighted picker. Both draw from the same actionsRoot candidate set. Generator string + // LabelSource selects how candidates are named to the model picker + // (verifier.LabelSourceVisibleText or verifier.LabelSourceResourceID). The + // seeded picker selects by index and never reads a label, so this reaches + // the model picker only. + LabelSource string } type Summary struct { diff --git a/internal/runner/source.go b/internal/runner/source.go index 44a3c75..bb66bac 100644 --- a/internal/runner/source.go +++ b/internal/runner/source.go @@ -107,6 +107,7 @@ func pickSources(options Options) (ActionSource, ExtractorSource, error) { client: client, model: config.Model, instructions: config.Instructions, + labelSource: options.LabelSource, logger: logger, history: newActionHistory(llmHistorySize), }