fix(runner): no silent generator fallback, and llm on web

--generator llm against a spec declaring no generator = llm(...) logged a
warning and ran the seeded picker. For a comparison campaign that is silent
arm corruption: the run completes, the directory looks correct, and the wrong
policy drove it. It is now fatal.

pickSources also returned the V8 source for both action and extractor on web
before it looked at the generator, so the llm policy was unreachable there.
The two axes are now independent: the driver picks the extractor source, the
flag picks the action source, and llmSource composes with either because the
runner populates the candidate list and screenshot on every platform.

Claude-Session: https://claude.ai/code/session_01A5KmftdEJ49A9z5mF5ESrX
This commit is contained in:
pj committed 2026-08-12 18:56:08 +05:30
1 parent 028534cadc
commit bee430cf72
2 files changed
+132 -26

No files matched your search

+96
View File
@@ -16,6 +16,8 @@ import (
"strings" "strings"
"testing" "testing"
"github.com/priyanshujain/sanderling/internal/driver"
mockdriver "github.com/priyanshujain/sanderling/internal/driver/mock"
"github.com/priyanshujain/sanderling/internal/hierarchy" "github.com/priyanshujain/sanderling/internal/hierarchy"
"github.com/priyanshujain/sanderling/internal/llmclient" "github.com/priyanshujain/sanderling/internal/llmclient"
"github.com/priyanshujain/sanderling/internal/verifier" "github.com/priyanshujain/sanderling/internal/verifier"
@@ -276,6 +278,100 @@ func TestPickSourcesSeededByDefault(t *testing.T) {
} }
} }
// seededFixtureSpec declares no generator = llm(...), so --generator llm has
// nothing to build a picker from.
const seededFixtureSpec = `
import { always, taps, typing, weighted } from "@sanderling/spec";
globalThis.properties = { ok: always(() => true) };
globalThis.actions = weighted([1, taps], [1, typing]);
`
func newSeededVerifier(t *testing.T) *verifier.Verifier {
t.Helper()
verifierInstance, err := verifier.New()
if err != nil {
t.Fatal(err)
}
if err := verifierInstance.Load(bundleSpec(t, seededFixtureSpec)); err != nil {
t.Fatal(err)
}
if _, ok := verifierInstance.LLMConfig(); ok {
t.Fatal("seeded fixture spec must not register an llm action backend")
}
return verifierInstance
}
// TestPickSourcesRejectsLLMWithoutSpecConfig pins the abort. Falling back to the
// seeded picker here completes the run, writes a well-formed output directory,
// and records it under the requested arm, so a campaign cell silently reports
// the wrong policy's numbers.
func TestPickSourcesRejectsLLMWithoutSpecConfig(t *testing.T) {
for name, activeDriver := range map[string]driver.DeviceDriver{
"native": nil,
"web": &webMockDriver{Driver: mockdriver.New()},
} {
t.Run(name, func(t *testing.T) {
action, extractor, err := pickSources(Options{
Driver: activeDriver,
Verifier: newSeededVerifier(t),
Generator: "llm",
Logger: slog.New(slog.NewTextHandler(io.Discard, nil)),
})
if err == nil {
t.Fatalf("pickSources = (%T, %T), want an error", action, extractor)
}
if action != nil || extractor != nil {
t.Errorf("sources = (%v, %v), want both nil alongside the error", action, extractor)
}
if !strings.Contains(err.Error(), "generator = llm(...)") {
t.Errorf("error = %q, want it to name the missing spec declaration", err)
}
})
}
}
// TestPickSourcesOnWebComposesLLMWithWebExtractors covers the second half of the
// same claim: the picker is chosen by --generator and the extractor source by
// the driver, so the llm policy runs on web instead of being silently replaced
// by the V8 picker.
func TestPickSourcesOnWebComposesLLMWithWebExtractors(t *testing.T) {
fake := newFakeOpenRouter(t)
_, verifierInstance := newLLMSource(t, fake)
action, extractor, err := pickSources(Options{
Driver: &webMockDriver{Driver: mockdriver.New()},
Verifier: verifierInstance,
Generator: "llm",
Logger: slog.New(slog.NewTextHandler(io.Discard, nil)),
})
if err != nil {
t.Fatalf("pickSources: %v", err)
}
if _, ok := action.(*llmSource); !ok {
t.Errorf("action source = %T, want *llmSource on web with --generator llm", action)
}
if _, ok := extractor.(webSource); !ok {
t.Errorf("extractor source = %T, want webSource so overrides still come from V8", extractor)
}
}
func TestPickSourcesOnWebSeededKeepsBothOnV8(t *testing.T) {
action, extractor, err := pickSources(Options{
Driver: &webMockDriver{Driver: mockdriver.New()},
Verifier: newSeededVerifier(t),
Generator: "seeded",
Logger: slog.New(slog.NewTextHandler(io.Discard, nil)),
})
if err != nil {
t.Fatalf("pickSources: %v", err)
}
if _, ok := action.(webSource); !ok {
t.Errorf("action source = %T, want webSource for the seeded web path", action)
}
if _, ok := extractor.(webSource); !ok {
t.Errorf("extractor source = %T, want webSource for the seeded web path", extractor)
}
}
func TestLLMSourceDrivesExecutedActions(t *testing.T) { func TestLLMSourceDrivesExecutedActions(t *testing.T) {
fake := newFakeOpenRouter(t) fake := newFakeOpenRouter(t)
source, verifierInstance := newLLMSource(t, fake) source, verifierInstance := newLLMSource(t, fake)
+36 -26
View File
@@ -3,6 +3,7 @@ package runner
import ( import (
"context" "context"
"encoding/json" "encoding/json"
"errors"
"fmt" "fmt"
"log/slog" "log/slog"
@@ -64,39 +65,48 @@ func (s webSource) ExtractorOverrides(ctx context.Context) (map[int]json.RawMess
// pickSources selects the runtime's action and extractor sources ONCE at setup // pickSources selects the runtime's action and extractor sources ONCE at setup
// from the driver's capabilities, the --generator flag, and the spec, so the // from the driver's capabilities, the --generator flag, and the spec, so the
// step loop never type-asserts. With --generator llm and a spec-declared // step loop never type-asserts.
// generator = llm({...}) it constructs the chat-completions client and returns //
// an llmSource for selection while extractor overrides still come from the goja // The two axes are independent. The DRIVER decides where extractor overrides
// path. Otherwise the seeded goja picker drives. // come from: the web path reads the values its extractors computed in V8, every
// other path has none. The --generator flag decides who picks the action, and
// the llm picker composes with either extractor source because it reads the
// goja-side candidate list and screenshot, both of which the runner populates on
// every platform.
//
// A missing generator = llm(...) is fatal rather than a fallback to the seeded
// picker. Falling back silently corrupts a comparison campaign: the run
// completes and the output directory looks correct while the wrong policy drove
// it.
func pickSources(options Options) (ActionSource, ExtractorSource, error) { func pickSources(options Options) (ActionSource, ExtractorSource, error) {
seeded := ActionSource(gojaSource{verifier: options.Verifier})
extractor := ExtractorSource(gojaSource{verifier: options.Verifier})
if web, ok := options.Driver.(driver.WebDriver); ok { if web, ok := options.Driver.(driver.WebDriver); ok {
source := webSource{web: web} source := webSource{web: web}
return source, source, nil seeded, extractor = source, source
}
if options.Generator != "llm" {
return seeded, extractor, nil
}
config, ok := options.Verifier.LLMConfig()
if !ok {
return nil, nil, errors.New("--generator llm: the spec declares no generator = llm(...); add one to the spec or drop --generator llm")
}
client, err := llmclient.New()
if err != nil {
return nil, nil, fmt.Errorf("llm action generator: %w", err)
} }
logger := options.Logger logger := options.Logger
if logger == nil { if logger == nil {
logger = slog.Default() logger = slog.Default()
} }
if options.Generator == "llm" { action := &llmSource{
config, ok := options.Verifier.LLMConfig() verifier: options.Verifier,
if !ok { client: client,
logger.Warn("--generator llm requested but spec declares no generator = llm(); using the seeded picker") model: config.Model,
} else { instructions: config.Instructions,
client, err := llmclient.New() logger: logger,
if err != nil { history: newActionHistory(llmHistorySize),
return nil, nil, fmt.Errorf("llm action generator: %w", err)
}
action := &llmSource{
verifier: options.Verifier,
client: client,
model: config.Model,
instructions: config.Instructions,
logger: logger,
history: newActionHistory(llmHistorySize),
}
return action, gojaSource{verifier: options.Verifier}, nil
}
} }
source := gojaSource{verifier: options.Verifier} return action, extractor, nil
return source, source, nil
} }