diff --git a/cmd/pad/server_info.go b/cmd/pad/server_info.go index 308206bcb..931389279 100644 --- a/cmd/pad/server_info.go +++ b/cmd/pad/server_info.go @@ -11,6 +11,7 @@ import ( "github.com/PerpetualSoftware/pad/internal/cli" "github.com/PerpetualSoftware/pad/internal/config" + "github.com/PerpetualSoftware/pad/internal/decision" ) type serverInfoReport struct { @@ -33,6 +34,30 @@ type serverInfoConfig struct { LoadedFromFile bool `json:"loaded_from_file"` LoadedFromEnv bool `json:"loaded_from_env"` LoadedFromFlags bool `json:"loaded_from_flags"` + + // DecisionProvider reports the resolved decision provider + // (PLAN-3114 / TASK-3116). + // + // It sits under `config` rather than at the top level because that is + // what it describes: THIS HOST's resolved configuration. The provider + // itself is constructed server-side, and this command's own help text + // promises it "does not inspect remote server internals" — so against a + // REMOTE server this reports the CLI host's settings, which may differ + // from the server's. Grouping it with the other local-config fields is + // what keeps that honest; promoting it to the top level would read as a + // statement about the server. + DecisionProvider serverInfoDecision `json:"decision_provider"` +} + +// serverInfoDecision is the resolved decision provider. +// +// It is always an OBJECT, with Name == "none" when nothing is configured, +// rather than alternating between an object and the bare string "none". A +// union-typed field would force every consumer to type-switch before reading +// a name, for no gain over a sentinel the name field can already carry. +type serverInfoDecision struct { + Name string `json:"name"` + Model string `json:"model,omitempty"` } type serverInfoConnection struct { @@ -99,6 +124,23 @@ does not inspect remote server internals.`, } } +// describeDecisionProvider resolves the decision provider for the report. +// +// The resolution order is the same one the server uses — config file first, +// environment last, because the environment overrides the instance-admin +// setting (ruled day 73 on PLAN-3114) — so this line cannot disagree with +// what the server would build from the same inputs. It goes through +// decision.Describe, which never returns the API key. +func describeDecisionProvider(cfg *config.Config) serverInfoDecision { + resolved := decision.Resolve(decision.Config{ + Provider: cfg.DecisionProvider, + APIKey: cfg.TypesafeAPIKey, + Model: cfg.DecisionModel, + }, decision.EnvConfig()) + name, model := decision.Describe(resolved) + return serverInfoDecision{Name: name, Model: model} +} + func collectServerInfo(cfg *config.Config) (*serverInfoReport, error) { report := &serverInfoReport{ Config: serverInfoConfig{ @@ -113,6 +155,7 @@ func collectServerInfo(cfg *config.Config) (*serverInfoReport, error) { LoadedFromFile: cfg.LoadedFromFile, LoadedFromEnv: cfg.LoadedFromEnv, LoadedFromFlags: cfg.LoadedFromFlags, + DecisionProvider: describeDecisionProvider(cfg), }, } @@ -243,6 +286,11 @@ func printServerInfo(report *serverInfoReport) { fmt.Fprintf(w, "Port:\t%d\n", report.Config.Port) } fmt.Fprintf(w, "Config source:\t%s\n", configSource(report.Config)) + if d := report.Config.DecisionProvider; d.Model != "" { + fmt.Fprintf(w, "Decision provider:\t%s (%s)\n", d.Name, d.Model) + } else { + fmt.Fprintf(w, "Decision provider:\t%s\n", d.Name) + } fmt.Println() fmt.Println("Connection") diff --git a/cmd/pad/server_info_decision_test.go b/cmd/pad/server_info_decision_test.go new file mode 100644 index 000000000..d80207ed5 --- /dev/null +++ b/cmd/pad/server_info_decision_test.go @@ -0,0 +1,176 @@ +package main + +import ( + "encoding/json" + "regexp" + "strings" + "testing" + + "github.com/PerpetualSoftware/pad/internal/config" + "github.com/PerpetualSoftware/pad/internal/decision" +) + +// These exercise the BINDING, not the component: internal/decision has its own +// tests for Resolve and Describe, and they have no opinion about whether +// `pad server info` ever calls them or with which precedence. Per CONVE-19, +// each case here asserts something only the wiring can get wrong. + +func TestDescribeDecisionProviderReportsNoneWhenUnconfigured(t *testing.T) { + t.Setenv("PAD_DECISION_PROVIDER", "") + t.Setenv("PAD_TYPESAFE_API_KEY", "") + t.Setenv("PAD_DECISION_MODEL", "") + + got := describeDecisionProvider(&config.Config{}) + if got.Name != decision.ProviderNone { + t.Errorf("Name = %q, want %q", got.Name, decision.ProviderNone) + } + if got.Model != "" { + t.Errorf("Model = %q, want empty", got.Model) + } +} + +func TestDescribeDecisionProviderReadsTheConfigFile(t *testing.T) { + t.Setenv("PAD_DECISION_PROVIDER", "") + t.Setenv("PAD_TYPESAFE_API_KEY", "") + t.Setenv("PAD_DECISION_MODEL", "") + + got := describeDecisionProvider(&config.Config{ + DecisionProvider: decision.ProviderTypesafe, + TypesafeAPIKey: "file-key", + DecisionModel: "jev-1.0.0", + }) + if got.Name != decision.ProviderTypesafe { + t.Errorf("Name = %q, want %q", got.Name, decision.ProviderTypesafe) + } + if got.Model != "jev-1.0.0" { + t.Errorf("Model = %q, want the config file's jev-1.0.0", got.Model) + } +} + +func TestDescribeDecisionProviderEnvOverridesTheFile(t *testing.T) { + // The precedence ruled on PLAN-3114 (day 73): the environment overrides + // the instance-admin setting, and the config file stands in for that + // source until TASK-3121 adds it. Only the wiring decides the ORDER the + // sources are passed in, so only a test here can catch it reversed. + t.Setenv("PAD_DECISION_PROVIDER", decision.ProviderTypesafe) + t.Setenv("PAD_TYPESAFE_API_KEY", "env-key") + t.Setenv("PAD_DECISION_MODEL", "jev-9.9.9") + + got := describeDecisionProvider(&config.Config{ + DecisionProvider: decision.ProviderTypesafe, + TypesafeAPIKey: "file-key", + DecisionModel: "jev-1.0.0", + }) + if got.Model != "jev-9.9.9" { + t.Errorf("Model = %q, want the environment's jev-9.9.9 to win over the file's jev-1.0.0", got.Model) + } +} + +func TestDescribeDecisionProviderEnvCanDisableTheFile(t *testing.T) { + t.Setenv("PAD_DECISION_PROVIDER", decision.ProviderNone) + t.Setenv("PAD_TYPESAFE_API_KEY", "") + t.Setenv("PAD_DECISION_MODEL", "") + + got := describeDecisionProvider(&config.Config{ + DecisionProvider: decision.ProviderTypesafe, + TypesafeAPIKey: "file-key", + }) + if got.Name != decision.ProviderNone { + t.Errorf("Name = %q, want %q: an explicit env 'none' must be able to disable what the file enabled", got.Name, decision.ProviderNone) + } +} + +func TestCollectedReportCarriesDecisionProviderAndNoKey(t *testing.T) { + // The report is what `--format json` serialises, so this is the shape a + // consumer sees. It also pins the absence of the API key, which is the + // one thing that must never reach this output. + t.Setenv("PAD_DECISION_PROVIDER", decision.ProviderTypesafe) + t.Setenv("PAD_TYPESAFE_API_KEY", "sk-super-secret-value") + t.Setenv("PAD_DECISION_MODEL", "") + + report := &serverInfoReport{Config: serverInfoConfig{ + DecisionProvider: describeDecisionProvider(&config.Config{}), + }} + + blob, err := json.Marshal(report) + if err != nil { + t.Fatalf("marshal report: %v", err) + } + s := string(blob) + + if !strings.Contains(s, `"decision_provider":{"name":"typesafe","model":"`+decision.DefaultModel+`"}`) { + t.Errorf("report JSON does not carry decision_provider with the pinned default model: %s", s) + } + if strings.Contains(s, "sk-super-secret-value") || strings.Contains(s, "secret") { + t.Errorf("report JSON leaked key material: %s", s) + } + + // Unconfigured still emits the object with a "none" name rather than + // omitting the field or emitting a bare string — a consumer must not + // have to type-switch. + t.Setenv("PAD_DECISION_PROVIDER", "") + t.Setenv("PAD_TYPESAFE_API_KEY", "") + blob, err = json.Marshal(&serverInfoReport{Config: serverInfoConfig{ + DecisionProvider: describeDecisionProvider(&config.Config{}), + }}) + if err != nil { + t.Fatalf("marshal report: %v", err) + } + if !strings.Contains(string(blob), `"decision_provider":{"name":"none"}`) { + t.Errorf("unconfigured report JSON = %s, want decision_provider {\"name\":\"none\"} with model omitted", blob) + } +} + +// Review round 2: the tests above call describeDecisionProvider directly, so +// they have no opinion about whether collectServerInfo calls it, or whether +// the human-readable output prints it. These go through both doors. +func TestServerInfoCarriesTheDecisionProviderThroughBothDoors(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + t.Setenv("PAD_DECISION_PROVIDER", decision.ProviderTypesafe) + t.Setenv("PAD_TYPESAFE_API_KEY", "sk-super-secret-value") + t.Setenv("PAD_DECISION_MODEL", "jev-9.9.9") + + cfg := config.DefaultConfig() + cfg.Mode = config.ModeRemote + // Nothing listens here; the connection probe fails fast and is recorded, + // which this test does not care about. + cfg.URL = "http://127.0.0.1:1" + + report, err := collectServerInfo(cfg) + if err != nil { + t.Fatalf("collectServerInfo: %v", err) + } + if got := report.Config.DecisionProvider; got.Name != decision.ProviderTypesafe || got.Model != "jev-9.9.9" { + t.Errorf("collectServerInfo reported decision provider %+v, want typesafe / jev-9.9.9", got) + } + + out := captureStdout(t, func() { printServerInfo(report) }) + if !regexp.MustCompile(`Decision provider:\s+typesafe \(jev-9\.9\.9\)`).MatchString(out) { + t.Errorf("human-readable output has no decision provider line:\n%s", out) + } + if strings.Contains(out, "secret") { + t.Errorf("human-readable output leaked key material:\n%s", out) + } + + // The printer must print the REPORT, not re-resolve the environment: a + // report carrying values the environment does not hold tells the two + // apart (review round 3). + report.Config.DecisionProvider = serverInfoDecision{Name: "report-only-name", Model: "report-only-model"} + out = captureStdout(t, func() { printServerInfo(report) }) + if !strings.Contains(out, "report-only-name (report-only-model)") { + t.Errorf("printServerInfo did not print the report's decision provider:\n%s", out) + } + + // Unconfigured: the line is still printed, and says none. + t.Setenv("PAD_DECISION_PROVIDER", "") + t.Setenv("PAD_TYPESAFE_API_KEY", "") + t.Setenv("PAD_DECISION_MODEL", "") + report, err = collectServerInfo(cfg) + if err != nil { + t.Fatalf("collectServerInfo: %v", err) + } + out = captureStdout(t, func() { printServerInfo(report) }) + if !regexp.MustCompile(`Decision provider:\s+none\n`).MatchString(out) { + t.Errorf("unconfigured output does not say the decision provider is none:\n%s", out) + } +} diff --git a/internal/config/config.go b/internal/config/config.go index 4a2b0f3ae..d0ddae15f 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -57,6 +57,24 @@ type Config struct { EmailFrom string `toml:"email_from"` // Sender address (e.g. noreply@getpad.dev) EmailFromName string `toml:"email_from_name"` // Sender display name (e.g. Pad) + // Decision provider (PLAN-3114 / TASK-3116). Empty or "none" disables + // decisions entirely, which is the default and is not a degraded mode. + // + // NOTE, because a reader will look for these in Load()'s env block + // below and not find them: the PAD_DECISION_PROVIDER / + // PAD_TYPESAFE_API_KEY / PAD_DECISION_MODEL environment variables are + // deliberately NOT applied here, unlike every other setting in this + // file. internal/decision owns exactly one resolver + // (decision.Resolve over decision.Config sources), so that the + // instance-admin setting arriving in TASK-3121 adds a SOURCE rather + // than a second place that reads provider settings. Applying the env + // here as well would make this file a second reader and would hide + // which source actually won. The fields below are the config-FILE + // source only. + DecisionProvider string `toml:"decision_provider"` + TypesafeAPIKey string `toml:"typesafe_api_key"` + DecisionModel string `toml:"decision_model"` + // Cloud mode CloudSecret string `toml:"cloud_secret"` // Inbound shared secret(s) accepted from pad-cloud. Comma-separated list supports rotation. CloudSidecarURL string `toml:"cloud_sidecar_url"` // Base URL pad uses to call the pad-cloud sidecar (reverse direction, e.g. Stripe cancel-customer on account delete) diff --git a/internal/decision/config.go b/internal/decision/config.go new file mode 100644 index 000000000..666e31a1b --- /dev/null +++ b/internal/decision/config.go @@ -0,0 +1,136 @@ +package decision + +import ( + "fmt" + "os" + "strings" +) + +// Provider names. +const ( + // ProviderTypesafe selects the typesafe.ai Jev backend. + ProviderTypesafe = "typesafe" + + // ProviderNone explicitly disables decisions. It exists so an + // environment variable can DISABLE a provider an instance-admin setting + // enabled: [Resolve] lets non-empty values win, and an empty string + // therefore means "this source has no opinion" rather than "off". See + // [Resolve] for why that distinction is load-bearing. + ProviderNone = "none" +) + +// DefaultModel is the pinned Jev model. +// +// It is a pinned VERSION rather than "jev-latest" deliberately: the provider +// documents that jev-latest moves without notice, and a decision model that +// changes underneath stored answers makes those answers uncomparable across +// time. Callers who want the moving target must set it explicitly. +const DefaultModel = "jev-1.13.0" + +// Config is the resolved decision-provider configuration. +// +// It is the SINGLE reader of provider settings. This unit populates it from +// the environment and from pad's config file; the instance-admin setting +// arrives in a later unit (TASK-3121) as another SOURCE passed to [Resolve], +// not as a second place that reads settings. See TASK-3116. +type Config struct { + // Provider is the backend name: "typesafe", "none", or "" when this + // source has no opinion. + Provider string + + // APIKey authenticates to the backend. Never logged, never printed, and + // never included in an error message. + APIKey string + + // Model pins the backend model. Empty falls back to [DefaultModel]. + Model string +} + +// Enabled reports whether this config selects a real provider. +func (c Config) Enabled() bool { + p := strings.TrimSpace(strings.ToLower(c.Provider)) + return p != "" && p != ProviderNone +} + +// EnvConfig reads the decision-provider settings from the environment. +// +// The variables follow the PAD_MAILEROO_API_KEY pattern in +// internal/config: PAD_DECISION_PROVIDER, PAD_TYPESAFE_API_KEY, +// PAD_DECISION_MODEL. +func EnvConfig() Config { + return Config{ + Provider: strings.TrimSpace(os.Getenv("PAD_DECISION_PROVIDER")), + APIKey: strings.TrimSpace(os.Getenv("PAD_TYPESAFE_API_KEY")), + Model: strings.TrimSpace(os.Getenv("PAD_DECISION_MODEL")), + } +} + +// Resolve merges configuration sources, LATER sources winning per field. +// Only non-empty fields override, so a source that says nothing about a +// field leaves the earlier value standing. +// +// Callers put the environment LAST, because the environment overrides the +// instance-admin setting (ruled day 73 on PLAN-3114). That precedence is why +// [ProviderNone] exists: since an empty string means "no opinion", an +// environment that wants to TURN OFF a provider an admin enabled has to say +// so with a value, and "none" is that value. +func Resolve(sources ...Config) Config { + var out Config + for _, s := range sources { + if v := strings.TrimSpace(s.Provider); v != "" { + out.Provider = v + } + if v := strings.TrimSpace(s.APIKey); v != "" { + out.APIKey = v + } + if v := strings.TrimSpace(s.Model); v != "" { + out.Model = v + } + } + return out +} + +// New builds the configured provider. +// +// A config that selects no provider returns (nil, nil) — the unconfigured +// case is not an error, and callers branch on a nil Provider. See the package +// doc. +// +// A config that names a provider but cannot satisfy it — an unknown name, or +// a missing API key — IS an error, because it describes an instance whose +// operator asked for decisions and will not get them. Failing quietly there +// would present a misconfiguration as the supported off state. +func New(cfg Config) (Provider, error) { + if !cfg.Enabled() { + return nil, nil + } + name := strings.ToLower(strings.TrimSpace(cfg.Provider)) + switch name { + case ProviderTypesafe: + if cfg.APIKey == "" { + return nil, fmt.Errorf("decision: provider %q is configured but PAD_TYPESAFE_API_KEY is empty", name) + } + model := cfg.Model + if model == "" { + model = DefaultModel + } + return newTypesafe(cfg.APIKey, model), nil + default: + return nil, fmt.Errorf("decision: unknown provider %q (known: %s, %s)", cfg.Provider, ProviderTypesafe, ProviderNone) + } +} + +// Describe reports the configured provider for operator-facing output such as +// `pad server info`. It returns ("none", "") when nothing is configured, and +// it NEVER returns the API key. +func Describe(cfg Config) (name, model string) { + if !cfg.Enabled() { + return ProviderNone, "" + } + name = strings.ToLower(strings.TrimSpace(cfg.Provider)) + model = cfg.Model + if model == "" && name == ProviderTypesafe { + model = DefaultModel + } + return name, model +} diff --git a/internal/decision/config_test.go b/internal/decision/config_test.go new file mode 100644 index 000000000..ab60f2108 --- /dev/null +++ b/internal/decision/config_test.go @@ -0,0 +1,205 @@ +package decision + +import ( + "strings" + "testing" +) + +func TestNewReturnsNilWhenUnconfigured(t *testing.T) { + // The unconfigured case is the DEFAULT and is not an error: callers + // branch on a nil Provider and behave exactly as they did before this + // package existed. + for _, cfg := range []Config{ + {}, + {Provider: ""}, + {Provider: " "}, + {Provider: ProviderNone}, + {Provider: "NONE"}, + // A key with no provider selected is still off — a stray + // PAD_TYPESAFE_API_KEY in the environment must not switch decisions + // on behind the operator's back. + {APIKey: "secret-key"}, + {Model: "jev-1.13.0"}, + } { + p, err := New(cfg) + if err != nil { + t.Errorf("New(%+v) returned an error for an unconfigured provider: %v", cfg, err) + } + if p != nil { + t.Errorf("New(%+v) returned a provider %T, want nil", cfg, p) + } + } +} + +func TestNewErrorsOnConfiguredButUnsatisfiable(t *testing.T) { + // The opposite of the above: an operator who ASKED for decisions and + // cannot get them must not be handed the silent off state. + t.Run("missing key", func(t *testing.T) { + p, err := New(Config{Provider: ProviderTypesafe}) + if err == nil { + t.Fatal("New succeeded with a provider selected and no API key") + } + if p != nil { + t.Error("New returned a provider alongside an error") + } + if strings.Contains(err.Error(), "secret") { + t.Error("error message may not carry key material") + } + }) + + t.Run("unknown provider", func(t *testing.T) { + _, err := New(Config{Provider: "acme", APIKey: "k"}) + if err == nil { + t.Fatal("New succeeded with an unknown provider name") + } + if !strings.Contains(err.Error(), "acme") { + t.Errorf("error does not name the unknown provider: %v", err) + } + }) +} + +func TestNewPinsTheModel(t *testing.T) { + p, err := New(Config{Provider: ProviderTypesafe, APIKey: "k"}) + if err != nil { + t.Fatalf("New: %v", err) + } + if p.Name() != ProviderTypesafe { + t.Errorf("Name() = %q, want %q", p.Name(), ProviderTypesafe) + } + // An unset model must resolve to the PINNED default, never to the + // provider's moving "latest" alias — a decision model that changes + // underneath stored answers makes them uncomparable over time. + if p.Model() != DefaultModel { + t.Errorf("Model() = %q, want the pinned %q", p.Model(), DefaultModel) + } + if strings.Contains(DefaultModel, "latest") { + t.Errorf("DefaultModel = %q, which is a moving alias", DefaultModel) + } + + p2, err := New(Config{Provider: ProviderTypesafe, APIKey: "k", Model: "jev-9.9.9"}) + if err != nil { + t.Fatalf("New: %v", err) + } + if p2.Model() != "jev-9.9.9" { + t.Errorf("Model() = %q, want the explicitly configured jev-9.9.9", p2.Model()) + } +} + +func TestResolvePrecedence(t *testing.T) { + file := Config{Provider: ProviderTypesafe, APIKey: "file-key", Model: "jev-1.0.0"} + + t.Run("later source wins per field", func(t *testing.T) { + got := Resolve(file, Config{APIKey: "env-key"}) + if got.APIKey != "env-key" { + t.Errorf("APIKey = %q, want env-key", got.APIKey) + } + // A source silent about a field must not blank it. + if got.Provider != ProviderTypesafe { + t.Errorf("Provider = %q, want the earlier source's value to survive", got.Provider) + } + if got.Model != "jev-1.0.0" { + t.Errorf("Model = %q, want the earlier source's value to survive", got.Model) + } + }) + + t.Run("empty does not clobber", func(t *testing.T) { + got := Resolve(file, Config{}, Config{Provider: " "}) + if got != file { + t.Errorf("Resolve = %+v, want the earlier config %+v unchanged", got, file) + } + }) + + t.Run("none disables what an earlier source enabled", func(t *testing.T) { + // This is why ProviderNone exists. With empty meaning "no opinion", + // an environment that wants to turn OFF a provider an admin enabled + // has to say so with a value. + got := Resolve(file, Config{Provider: ProviderNone}) + if got.Enabled() { + t.Error("Resolve kept the provider enabled after a later source said none") + } + p, err := New(got) + if err != nil || p != nil { + t.Errorf("New after an explicit none = (%v, %v), want (nil, nil)", p, err) + } + }) + + t.Run("no sources", func(t *testing.T) { + if got := Resolve(); got.Enabled() { + t.Error("Resolve() with no sources reported enabled") + } + }) +} + +func TestEnvConfigReadsAndTrims(t *testing.T) { + t.Setenv("PAD_DECISION_PROVIDER", " typesafe ") + t.Setenv("PAD_TYPESAFE_API_KEY", " env-key ") + t.Setenv("PAD_DECISION_MODEL", " jev-2.0.0 ") + + got := EnvConfig() + want := Config{Provider: ProviderTypesafe, APIKey: "env-key", Model: "jev-2.0.0"} + if got != want { + t.Errorf("EnvConfig() = %+v, want %+v", got, want) + } +} + +func TestEnvConfigEmptyWhenUnset(t *testing.T) { + t.Setenv("PAD_DECISION_PROVIDER", "") + t.Setenv("PAD_TYPESAFE_API_KEY", "") + t.Setenv("PAD_DECISION_MODEL", "") + if got := EnvConfig(); got != (Config{}) { + t.Errorf("EnvConfig() = %+v, want the zero Config so it cannot clobber another source", got) + } +} + +func TestDescribeNeverLeaksTheKey(t *testing.T) { + const key = "sk-super-secret-value" + + name, model := Describe(Config{Provider: ProviderTypesafe, APIKey: key}) + if name != ProviderTypesafe { + t.Errorf("name = %q, want %q", name, ProviderTypesafe) + } + if model != DefaultModel { + t.Errorf("model = %q, want the pinned default %q", model, DefaultModel) + } + if strings.Contains(name+model, key) || strings.Contains(name+model, "secret") { + t.Errorf("Describe leaked key material: name=%q model=%q", name, model) + } + + // Unconfigured reports the sentinel and no model, which is what + // `pad server info` prints. + name, model = Describe(Config{}) + if name != ProviderNone { + t.Errorf("name = %q, want %q", name, ProviderNone) + } + if model != "" { + t.Errorf("model = %q, want empty when nothing is configured", model) + } + + // An unknown provider is still described rather than reported as none: + // saying "none" for a misconfiguration would hide it from the operator + // looking at exactly this output. + name, _ = Describe(Config{Provider: "acme", APIKey: key}) + if name != "acme" { + t.Errorf("name = %q, want the configured-but-unknown name reported as-is", name) + } +} + +func TestEnabled(t *testing.T) { + for _, tt := range []struct { + cfg Config + want bool + }{ + {Config{}, false}, + {Config{Provider: ""}, false}, + {Config{Provider: " "}, false}, + {Config{Provider: ProviderNone}, false}, + {Config{Provider: "None"}, false}, + {Config{Provider: ProviderTypesafe}, true}, + {Config{Provider: "TypeSafe"}, true}, + {Config{Provider: "acme"}, true}, // configured, even if unsatisfiable + } { + if got := tt.cfg.Enabled(); got != tt.want { + t.Errorf("Config{Provider:%q}.Enabled() = %v, want %v", tt.cfg.Provider, got, tt.want) + } + } +} diff --git a/internal/decision/decision.go b/internal/decision/decision.go new file mode 100644 index 000000000..2bb018c55 --- /dev/null +++ b/internal/decision/decision.go @@ -0,0 +1,308 @@ +// Package decision provides a pluggable typed-decision provider: a small set +// of classification primitives (Choice, Score, Noul) answered in one request +// (retried on transient failure), with calibrated probabilities where the +// primitive has them. +// +// The package is a PROVIDER INTERFACE, not an integration with any one +// vendor. The only backend today is typesafe.ai's Jev model (see +// typesafe.go), selected by configuration. +// +// # Unconfigured means nil +// +// When no provider is configured, [New] returns a nil [Provider] and no +// error. Callers treat nil as "no decisions available" and behave exactly as +// they did before this package existed — there are deliberately no no-op +// implementations and no degraded paths, so that the absence of a provider is +// a visible branch at every call site rather than a silent difference in +// behaviour. See PLAN-3114 and TASK-3116. +package decision + +import ( + "context" + "errors" + "fmt" + "sort" + "strings" +) + +// Kind names one of the three decision primitives. +// +// The wire values are the provider's, and the Go constants deliberately +// match them so a decoded answer needs no translation table. +type Kind string + +const ( + // KindChoice picks exactly one of a set of named options, and returns a + // probability per option plus a confidence. + KindChoice Kind = "choice" + + // KindScore places the state on an ORDERED scale of levels, and returns + // a probability per level plus a confidence. See [Answer.Score] for the + // one genuinely surprising thing about this primitive. + KindScore Kind = "score" + + // KindNoul returns the probability that a statement is true. It carries + // NO confidence and NO probabilities — see [Answer.Confidence]. + KindNoul Kind = "noul" +) + +// Question is one thing to ask about a state. Construct one with [Choice], +// [Score] or [Noul] rather than by literal: the three primitives take +// mutually incompatible criteria (a map, an ordered slice, and a pair of +// descriptions), so a hand-built literal can express a Choice carrying +// Score's levels, which no provider can answer. +type Question struct { + Kind Kind + + // Instructions is the question in plain language. + Instructions string + + // Options is Choice's criteria: option name -> what that option means. + // Empty for the other kinds. + Options map[string]string + + // Levels is Score's criteria: an ORDERED slice, lowest first. The order + // is the scale, so it is part of the value and not a presentation + // detail. Empty for the other kinds. + Levels []string + + // TrueDesc and FalseDesc are Noul's optional criteria, describing what + // makes the statement true and false respectively. Both empty is valid: + // the provider then works from Instructions alone. + TrueDesc, FalseDesc string +} + +// Choice builds a Choice question. options maps each option name to a +// description of what that option means; at least two are required. +func Choice(instructions string, options map[string]string) Question { + return Question{Kind: KindChoice, Instructions: instructions, Options: options} +} + +// Score builds a Score question over an ORDERED scale, lowest level first. +// At least two levels are required. +func Score(instructions string, levels []string) Question { + return Question{Kind: KindScore, Instructions: instructions, Levels: levels} +} + +// Noul builds a Noul question — the probability that a statement is true. +// trueDesc and falseDesc are optional; pass "" for both to work from the +// instructions alone. +func Noul(instructions, trueDesc, falseDesc string) Question { + return Question{Kind: KindNoul, Instructions: instructions, TrueDesc: trueDesc, FalseDesc: falseDesc} +} + +// maxChoiceOptions is the provider's documented ceiling on Choice options. +const maxChoiceOptions = 255 + +// minLevels is the provider's documented floor on Score levels. +const minLevels = 2 + +// Validate reports whether the question is answerable as constructed. +// +// It rejects criteria belonging to a DIFFERENT kind as well as missing ones: +// a Choice carrying Levels is a caller who meant to call [Score], and +// silently dropping the levels would send a question the caller did not +// write. +func (q Question) Validate() error { + if strings.TrimSpace(q.Instructions) == "" { + return errors.New("decision: question instructions are empty") + } + switch q.Kind { + case KindChoice: + if len(q.Options) < 2 { + return fmt.Errorf("decision: choice question needs at least 2 options, got %d", len(q.Options)) + } + if len(q.Options) > maxChoiceOptions { + return fmt.Errorf("decision: choice question has %d options, limit is %d", len(q.Options), maxChoiceOptions) + } + for name, desc := range q.Options { + if strings.TrimSpace(name) == "" { + return errors.New("decision: choice question has an empty option name") + } + if strings.TrimSpace(desc) == "" { + return fmt.Errorf("decision: choice option %q has an empty description", name) + } + } + if len(q.Levels) > 0 || q.TrueDesc != "" || q.FalseDesc != "" { + return errors.New("decision: choice question carries another kind's criteria (levels or true/false)") + } + case KindScore: + if len(q.Levels) < minLevels { + return fmt.Errorf("decision: score question needs at least %d levels, got %d", minLevels, len(q.Levels)) + } + for i, l := range q.Levels { + if strings.TrimSpace(l) == "" { + return fmt.Errorf("decision: score level %d is empty", i) + } + } + if len(q.Options) > 0 || q.TrueDesc != "" || q.FalseDesc != "" { + return errors.New("decision: score question carries another kind's criteria (options or true/false)") + } + case KindNoul: + if len(q.Options) > 0 || len(q.Levels) > 0 { + return errors.New("decision: noul question carries another kind's criteria (options or levels)") + } + // An asymmetric pair is almost certainly a mistake — the provider + // reads criteria literally, so describing only one side biases the + // answer toward the side that was described. + if (q.TrueDesc == "") != (q.FalseDesc == "") { + return errors.New("decision: noul question describes only one of true/false; give both or neither") + } + default: + return fmt.Errorf("decision: unknown question kind %q", q.Kind) + } + return nil +} + +// Answer is one question's answer. Which members are populated is decided by +// [Answer.Kind]; reading a member belonging to another kind is a caller bug +// that the zero value will not announce, which is why [Answer.Confidence] is +// a pointer. +type Answer struct { + // Kind echoes the primitive that produced this answer. + Kind Kind + + // Choice is the selected option name, for [KindChoice] only. + Choice string + + // Score is the position on the scale, for [KindScore] only. + // + // IT IS NOT A LEVEL INDEX. The provider returns the + // probability-weighted MEAN over level indices, so it is a float that + // usually falls BETWEEN levels: a measured answer with + // probabilities {"0":0.86,"1":0.14} returned score 0.14, which is + // 0.86*0 + 0.14*1 and not the index of any level. To recover "which + // level did it pick", take the argmax of Probabilities + // ([Answer.TopLevel]); truncating or int-casting Score gives the wrong + // level for any distribution whose mass does not sit on index 0. + // (Measured against jev-1.13.0; the arithmetic is on TASK-3116's trail.) + Score float64 + + // Noul is the probability that the statement is true, for [KindNoul] + // only. Note that this IS the answer — a Noul carries no separate + // confidence, so the distance from 0.5 is all the certainty there is. + Noul float64 + + // Legend maps a stringified level index to that level's name, for + // [KindScore] only. The provider echoes the Levels that were sent, so a + // stored answer stays interpretable after the question's levels are + // reworded. Its KEYS are checked against the levels sent; its names are + // echoed text and are not. + Legend map[string]string + + // Probabilities is the distribution over outcomes. THE KEY VOCABULARY + // DEPENDS ON KIND: option NAMES for [KindChoice], stringified level + // INDICES ("0", "1", ...) for [KindScore]. Nil for [KindNoul], which + // has no distribution. A consumer that assumes one vocabulary and is + // handed the other reads a missing key as 0.0, so switch on Kind. + Probabilities map[string]float64 + + // Confidence is how sure the provider is, for [KindChoice] and + // [KindScore]. + // + // It is a POINTER because [KindNoul] answers carry no confidence at + // all, and 0.0 is not a neutral stand-in for "absent" — it is the most + // extreme value the scale has. A caller following the provider's own + // recommended banding (act above 0.9, route to a human below 0.5) would + // read every Noul answer as minimum confidence and route all of them to + // a human, which states the opposite of the truth that the primitive + // has no opinion on confidence. A nil pointer forces that check. + Confidence *float64 +} + +// ConfidenceOr returns the answer's confidence, or def when the primitive +// carries none. Use it where a missing confidence has a defensible default; +// read [Answer.Confidence] directly where it does not. +func (a Answer) ConfidenceOr(def float64) float64 { + if a.Confidence == nil { + return def + } + return *a.Confidence +} + +// TopLevel returns the highest-probability level of a Score answer: its index +// and its name from the legend. ok is false for any other kind, or when the +// answer carries no probabilities. +// +// An answer returned by [Provider.Ask] has had every probability key checked +// against the levels sent. An Answer built by hand has not: a key that does +// not parse is treated as a level with a very large index and whatever name +// the legend holds for it (often ""), and still yields ok=true. +// +// This is the correct way to ask "which level did it pick" — see +// [Answer.Score] for why that field is not it. +// +// The result is deterministic for ANY key set, not only for well-formed ones. +// Ties on probability resolve to the lowest level index, and ties on index +// resolve to the lexicographically smaller key — the second tie-break is +// needed because distinct key STRINGS can share an index ("1" and "01" both +// parse to 1) and because every unparseable key shares the same sentinel, so +// ordering on index alone leaves those cases decided by Go's randomised map +// iteration order. +func (a Answer) TopLevel() (index int, name string, ok bool) { + if a.Kind != KindScore || len(a.Probabilities) == 0 { + return 0, "", false + } + keys := make([]string, 0, len(a.Probabilities)) + for k := range a.Probabilities { + keys = append(keys, k) + } + sort.Slice(keys, func(i, j int) bool { + li, lj := levelIndex(keys[i]), levelIndex(keys[j]) + if li != lj { + return li < lj + } + return keys[i] < keys[j] + }) + + best := keys[0] + for _, k := range keys[1:] { + if a.Probabilities[k] > a.Probabilities[best] { + best = k + } + } + return levelIndex(best), a.Legend[best], true +} + +// Usage reports what one [Provider.Ask] call consumed and whether the +// provider had to shorten the state to fit. +type Usage struct { + InputTokens int `json:"input_tokens"` + OutputTokens int `json:"output_tokens"` + + // StateTruncated reports that the state was shortened at the provider + // boundary to fit the backend's token budget, so the answers describe + // LESS than the caller supplied. + // + // It lives on Usage rather than in its own return value because + // [Provider.Ask]'s signature is fixed by TASK-3116's contract, and + // Usage is already the "what did this call actually do" struct. A + // caller acting on a truncated answer should say so wherever it + // records the result. + StateTruncated bool `json:"state_truncated,omitempty"` +} + +// Provider answers typed questions about a state. +// +// A nil Provider is the configured-off case and is not an error; see the +// package doc. +type Provider interface { + // Name is the backend's configuration name, e.g. "typesafe". + Name() string + + // Model is the pinned model identifier this provider will use. + Model() string + + // Ask answers every question about the same state in ONE request, + // retried on transient provider failures. + // + // state is marshalled as JSON, so a string, a map, or any + // JSON-marshallable struct is accepted; prefer the narrowest state that + // can answer the questions, because irrelevant state degrades accuracy. + // + // The returned map has one entry per question key. An error means NO + // answers: this is deliberately all-or-nothing, since a partial map + // would make "the provider had no opinion" indistinguishable from "the + // call half failed". + Ask(ctx context.Context, state any, questions map[string]Question) (map[string]Answer, Usage, error) +} diff --git a/internal/decision/decision_test.go b/internal/decision/decision_test.go new file mode 100644 index 000000000..0dd006406 --- /dev/null +++ b/internal/decision/decision_test.go @@ -0,0 +1,706 @@ +package decision + +import ( + "context" + "encoding/json" + "errors" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + "unicode/utf8" +) + +// fake stands up an httptest server speaking the provider's protocol and +// returns a provider pointed at it. +// +// handler receives the decoded request so a test can assert on the WIRE SHAPE +// the provider produced, not merely on what it decoded back — the two are +// different claims and only the first catches an encoder that agrees with its +// own decoder. +type fake struct { + t *testing.T + srv *httptest.Server + requests []wireRequest + // raw bodies as received, for tests asserting on criteria shape where + // the typed wireRequest would have normalised it away. + rawBodies []string + slept []time.Duration +} + +func newFake(t *testing.T, handler func(f *fake, req wireRequest, raw []byte, w http.ResponseWriter)) (*typesafeProvider, *fake) { + t.Helper() + f := &fake{t: t} + f.srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if got := r.Header.Get("Authorization"); got != "Bearer test-key" { + t.Errorf("Authorization header = %q, want Bearer test-key", got) + } + if got := r.Header.Get("Content-Type"); got != "application/json" { + t.Errorf("Content-Type = %q, want application/json", got) + } + // io.ReadAll, not a single Read into a ContentLength-sized slice: a + // single Read may return fewer bytes, which would truncate the body + // and make every request-shape assertion below fail for the wrong + // reason. + raw, err := io.ReadAll(r.Body) + if err != nil { + t.Errorf("fake could not read request body: %v", err) + } + var req wireRequest + if err := json.Unmarshal(raw, &req); err != nil { + t.Errorf("fake could not decode request: %v (body %q)", err, string(raw)) + } + f.requests = append(f.requests, req) + f.rawBodies = append(f.rawBodies, string(raw)) + handler(f, req, raw, w) + })) + t.Cleanup(f.srv.Close) + + p := newTypesafe("test-key", "jev-1.13.0") + p.setEndpoint(f.srv.URL) + // Record the requested waits without spending wall clock, but keep the + // context's own semantics: a cancelled context must still abort the wait, + // or a test double would hide the very behaviour it is standing in for. + p.sleep = func(ctx context.Context, d time.Duration) error { + f.slept = append(f.slept, d) + return ctx.Err() + } + return p, f +} + +func ok(body string) func(*fake, wireRequest, []byte, http.ResponseWriter) { + return func(_ *fake, _ wireRequest, _ []byte, w http.ResponseWriter) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(body)) + } +} + +// --------------------------------------------------------------------------- +// Shape round-trip, one test per primitive. +// +// Every response body below is the VERBATIM JSON recorded from +// api.typesafe.ai/v1/systemone against jev-1.13.0 (choice and noul from the +// 760-call eval run, score from this unit's probe — receipts on TASK-3116's +// trail). They are pinned as literals rather than produced by marshalling our +// own wire structs, so a test cannot pass by agreeing with a wrong encoder. +// --------------------------------------------------------------------------- + +const recordedChoiceResponse = `{"model":"jev-1.13.0","answers":{"severity":{"type":"choice","choice":"high","confidence":0.63,"probabilities":{"high":0.72,"critical":0.05,"medium":0.22,"low":0.01}}},"usage":{"input_tokens":1138,"output_tokens":165}}` + +const recordedNoulResponse = `{"model":"jev-1.13.0","answers":{"needs_human_decision":{"type":"noul","noul":0.92}},"usage":{"input_tokens":1224,"output_tokens":41}}` + +const recordedScoreResponse = `{"model":"jev-1.13.0","answers":{"severity":{"type":"score","score":0.14,"confidence":0.88,"legend":{"0":"cosmetic only","1":"annoying but harmless","2":"a real defect in normal use","3":"breaks a core flow","4":"data loss or security exposure"},"probabilities":{"0":0.86,"1":0.14,"2":0.0,"3":0.0,"4":0.0}}},"usage":{"input_tokens":372,"output_tokens":17}}` + +func TestAskChoiceRoundTrip(t *testing.T) { + p, f := newFake(t, ok(recordedChoiceResponse)) + + q := Choice("How severe is this bug?", map[string]string{ + "low": "cosmetic", + "medium": "a real defect", + "high": "breaks a core flow", + "critical": "data loss", + }) + answers, usage, err := p.Ask(context.Background(), "a bug report", map[string]Question{"severity": q}) + if err != nil { + t.Fatalf("Ask: %v", err) + } + + // Request shape: choice criteria is an OBJECT of option -> description. + raw := f.rawBodies[0] + if !strings.Contains(raw, `"type":"choice"`) { + t.Errorf("request did not carry type choice: %s", raw) + } + if !strings.Contains(raw, `"criteria":{`) { + t.Errorf("choice criteria must serialise as an object, got: %s", raw) + } + if f.requests[0].Model != "jev-1.13.0" { + t.Errorf("model = %q, want the pinned jev-1.13.0", f.requests[0].Model) + } + + a := answers["severity"] + if a.Kind != KindChoice { + t.Errorf("Kind = %q, want %q", a.Kind, KindChoice) + } + if a.Choice != "high" { + t.Errorf("Choice = %q, want high", a.Choice) + } + if a.Confidence == nil { + t.Fatal("Confidence is nil; a choice answer carries one") + } + if *a.Confidence != 0.63 { + t.Errorf("Confidence = %v, want 0.63", *a.Confidence) + } + if got := a.Probabilities["high"]; got != 0.72 { + t.Errorf("Probabilities[high] = %v, want 0.72", got) + } + // Choice probabilities are keyed by OPTION NAME, never by index. A + // consumer written for score's vocabulary must not find a key here. + if _, hasIndexKey := a.Probabilities["0"]; hasIndexKey { + t.Error(`choice probabilities must not be keyed by level index`) + } + if a.Legend != nil { + t.Errorf("Legend = %v, want nil for a choice answer", a.Legend) + } + if usage.InputTokens != 1138 || usage.OutputTokens != 165 { + t.Errorf("Usage = %+v, want 1138/165", usage) + } + if usage.StateTruncated { + t.Error("StateTruncated is true for a short state") + } +} + +func TestAskNoulRoundTripCarriesNoConfidence(t *testing.T) { + p, f := newFake(t, ok(recordedNoulResponse)) + + q := Noul("The item is waiting on a human's judgment.", + "a human must decide", "an agent can proceed") + answers, _, err := p.Ask(context.Background(), "an item body", map[string]Question{"needs_human_decision": q}) + if err != nil { + t.Fatalf("Ask: %v", err) + } + + raw := f.rawBodies[0] + if !strings.Contains(raw, `"type":"noul"`) { + t.Errorf("request did not carry type noul: %s", raw) + } + if !strings.Contains(raw, `"true":"a human must decide"`) { + t.Errorf("noul criteria must carry the true/false pair: %s", raw) + } + + a := answers["needs_human_decision"] + if a.Kind != KindNoul { + t.Errorf("Kind = %q, want %q", a.Kind, KindNoul) + } + if a.Noul != 0.92 { + t.Errorf("Noul = %v, want 0.92", a.Noul) + } + + // THE point of the pointer. Asserting only "Confidence is nil" would + // also pass if the field were a float64 that happened to be + // unset — which is exactly the bug — so assert the CONSEQUENCE a caller + // would see: the banding default must be the caller's, not 0.0. + if a.Confidence != nil { + t.Errorf("Confidence = %v, want nil: a noul answer carries none", *a.Confidence) + } + if got := a.ConfidenceOr(1.0); got != 1.0 { + t.Errorf("ConfidenceOr(1.0) = %v, want the caller's default 1.0; a zero here "+ + "means an absent confidence is being read as minimum confidence", got) + } + if a.Probabilities != nil { + t.Errorf("Probabilities = %v, want nil for a noul answer", a.Probabilities) + } +} + +func TestAskScoreRoundTripAndScoreIsNotALevelIndex(t *testing.T) { + p, f := newFake(t, ok(recordedScoreResponse)) + + levels := []string{ + "cosmetic only", "annoying but harmless", "a real defect in normal use", + "breaks a core flow", "data loss or security exposure", + } + answers, _, err := p.Ask(context.Background(), "a bug report", map[string]Question{ + "severity": Score("Rate how severe this bug is.", levels), + }) + if err != nil { + t.Fatalf("Ask: %v", err) + } + + // Request shape: score criteria is an ORDERED ARRAY, not a map. This is + // the one request-shape difference that a map would silently survive + // locally and fail against the live API. + raw := f.rawBodies[0] + if !strings.Contains(raw, `"criteria":["cosmetic only","annoying but harmless"`) { + t.Errorf("score criteria must serialise as an ordered array in the given order, got: %s", raw) + } + + a := answers["severity"] + if a.Kind != KindScore { + t.Errorf("Kind = %q, want %q", a.Kind, KindScore) + } + if a.Score != 0.14 { + t.Errorf("Score = %v, want 0.14", a.Score) + } + if a.Legend["4"] != "data loss or security exposure" { + t.Errorf("Legend[4] = %q, want the highest level", a.Legend["4"]) + } + if got := a.Probabilities["0"]; got != 0.86 { + t.Errorf("Probabilities[0] = %v, want 0.86", got) + } + + // Score is the probability-weighted MEAN over level indices, so the + // picked level comes from the argmax and NOT from the number. + idx, name, okTop := a.TopLevel() + if !okTop { + t.Fatal("TopLevel() reported no level for a score answer") + } + if idx != 0 || name != "cosmetic only" { + t.Errorf("TopLevel() = (%d, %q), want (0, cosmetic only)", idx, name) + } + // The counterfactual that makes the assertion above mean something: the + // mass here sits on index 0, and 0.14 rounds to 0 too, so this case + // alone cannot distinguish argmax from rounding. Assert on a + // distribution where they DISAGREE. + skewed := Answer{ + Kind: KindScore, + Score: 2.6, // 0.4*2 + 0.6*3 + Legend: map[string]string{"0": "a", "1": "b", "2": "c", "3": "d"}, + Probabilities: map[string]float64{"0": 0, "1": 0, "2": 0.4, "3": 0.6}, + } + if idx, name, _ := skewed.TopLevel(); idx != 3 || name != "d" { + t.Errorf("TopLevel() on a skewed distribution = (%d, %q), want (3, d); "+ + "int-casting Score would give 2 here", idx, name) + } +} + +func TestTopLevelTiesResolveToLowestIndexAndRejectOtherKinds(t *testing.T) { + tie := Answer{ + Kind: KindScore, + Legend: map[string]string{"0": "low", "1": "high"}, + Probabilities: map[string]float64{"0": 0.5, "1": 0.5}, + } + // Run repeatedly: a map-order-dependent implementation passes this + // intermittently, so one iteration would not discriminate. + for i := 0; i < 50; i++ { + if idx, name, _ := tie.TopLevel(); idx != 0 || name != "low" { + t.Fatalf("iteration %d: TopLevel() = (%d, %q), want the lowest index (0, low)", i, idx, name) + } + } + if _, _, okTop := (Answer{Kind: KindChoice, Probabilities: map[string]float64{"a": 1}}).TopLevel(); okTop { + t.Error("TopLevel() reported ok for a choice answer") + } + if _, _, okTop := (Answer{Kind: KindScore}).TopLevel(); okTop { + t.Error("TopLevel() reported ok for a score answer with no probabilities") + } +} + +// --------------------------------------------------------------------------- +// Errors +// --------------------------------------------------------------------------- + +func TestOversizedStateSurfacesTypedError(t *testing.T) { + // The body is the one measured from the live API. + p, _ := newFake(t, func(_ *fake, _ wireRequest, _ []byte, w http.ResponseWriter) { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"detail":{"error_type":"max_tokens_exceeded"}}`)) + }) + + _, _, err := p.Ask(context.Background(), "s", map[string]Question{ + "q": Noul("a statement", "", ""), + }) + if err == nil { + t.Fatal("Ask succeeded on a 400 max_tokens_exceeded") + } + if !errors.Is(err, ErrMaxTokensExceeded) { + t.Errorf("errors.Is(err, ErrMaxTokensExceeded) = false for a measured budget refusal; err = %v", err) + } + var apiErr *APIError + if !errors.As(err, &apiErr) { + t.Fatalf("err is not an *APIError: %v", err) + } + if apiErr.StatusCode != http.StatusBadRequest { + t.Errorf("StatusCode = %d, want 400 (the MEASURED status; the contract said 422)", apiErr.StatusCode) + } + if apiErr.ErrorType != "max_tokens_exceeded" { + t.Errorf("ErrorType = %q, want max_tokens_exceeded", apiErr.ErrorType) + } +} + +func Test422SurfacesBodyAndIsNotABudgetError(t *testing.T) { + const body = `{"detail":[{"loc":["body","questions","q","criteria"],"msg":"at least 2 levels required"}]}` + p, _ := newFake(t, func(_ *fake, _ wireRequest, _ []byte, w http.ResponseWriter) { + w.WriteHeader(http.StatusUnprocessableEntity) + _, _ = w.Write([]byte(body)) + }) + + _, _, err := p.Ask(context.Background(), "s", map[string]Question{"q": Noul("a statement", "", "")}) + if err == nil { + t.Fatal("Ask succeeded on a 422") + } + var apiErr *APIError + if !errors.As(err, &apiErr) { + t.Fatalf("err is not an *APIError: %v", err) + } + if apiErr.Body != body { + t.Errorf("Body = %q, want the provider's body verbatim %q", apiErr.Body, body) + } + if !strings.Contains(err.Error(), "at least 2 levels required") { + t.Errorf("Error() dropped the provider's message, which names the offending question: %q", err.Error()) + } + // Control leg: 422 must NOT match the budget sentinel, or the typed + // error above would be meaningless. + if errors.Is(err, ErrMaxTokensExceeded) { + t.Error("a 422 validation failure matched ErrMaxTokensExceeded") + } +} + +func TestRetriesTransientThenSucceeds(t *testing.T) { + calls := 0 + p, f := newFake(t, func(_ *fake, _ wireRequest, _ []byte, w http.ResponseWriter) { + calls++ + switch calls { + case 1: + w.WriteHeader(http.StatusTooManyRequests) + _, _ = w.Write([]byte(`{"detail":{"error_type":"rate_limited"}}`)) + case 2: + w.WriteHeader(529) + _, _ = w.Write([]byte(`{"detail":{"error_type":"overloaded"}}`)) + default: + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(recordedNoulResponse)) + } + }) + + answers, _, err := p.Ask(context.Background(), "s", map[string]Question{ + "needs_human_decision": Noul("a statement", "", ""), + }) + if err != nil { + t.Fatalf("Ask: %v", err) + } + if calls != 3 { + t.Errorf("provider was called %d times, want 3 (429, 529, then 200)", calls) + } + if answers["needs_human_decision"].Noul != 0.92 { + t.Errorf("answer not returned after a successful retry: %+v", answers) + } + // Assert the backoff GREW, which is what distinguishes exponential + // backoff from a fixed sleep — the end state (a success) is identical + // either way. + if len(f.slept) != 2 { + t.Fatalf("slept %d times, want 2: %v", len(f.slept), f.slept) + } + if f.slept[0] != initialBackoff { + t.Errorf("first backoff = %v, want %v", f.slept[0], initialBackoff) + } + if f.slept[1] <= f.slept[0] { + t.Errorf("backoff did not grow: %v then %v", f.slept[0], f.slept[1]) + } +} + +func TestRetryAfterHeaderOverridesBackoffAndIsBounded(t *testing.T) { + calls := 0 + p, f := newFake(t, func(_ *fake, _ wireRequest, _ []byte, w http.ResponseWriter) { + calls++ + if calls == 1 { + w.Header().Set("Retry-After", "3") + w.WriteHeader(http.StatusTooManyRequests) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(recordedNoulResponse)) + }) + if _, _, err := p.Ask(context.Background(), "s", map[string]Question{ + "needs_human_decision": Noul("a statement", "", ""), + }); err != nil { + t.Fatalf("Ask: %v", err) + } + if len(f.slept) != 1 || f.slept[0] != 3*time.Second { + t.Errorf("slept %v, want one 3s wait from the Retry-After header", f.slept) + } + + if got := parseRetryAfter("99999"); got != maxRetryAfter { + t.Errorf("parseRetryAfter(99999) = %v, want it clamped to %v", got, maxRetryAfter) + } + for _, bad := range []string{"", "0", "-5", "Wed, 21 Oct 2015 07:28:00 GMT", "soon"} { + if got := parseRetryAfter(bad); got != 0 { + t.Errorf("parseRetryAfter(%q) = %v, want 0 so the normal backoff applies", bad, got) + } + } +} + +func TestGivesUpAfterMaxAttempts(t *testing.T) { + calls := 0 + p, _ := newFake(t, func(_ *fake, _ wireRequest, _ []byte, w http.ResponseWriter) { + calls++ + w.WriteHeader(http.StatusTooManyRequests) + }) + _, _, err := p.Ask(context.Background(), "s", map[string]Question{"q": Noul("a statement", "", "")}) + if err == nil { + t.Fatal("Ask succeeded against a provider that only ever 429s") + } + if calls != maxAttempts { + t.Errorf("provider was called %d times, want maxAttempts=%d", calls, maxAttempts) + } +} + +func TestPartialAnswerSetIsAnError(t *testing.T) { + // Two questions asked, one answered. A partial map would leave the + // caller unable to tell "no opinion" from "half the call failed". + p, _ := newFake(t, ok(recordedNoulResponse)) + _, _, err := p.Ask(context.Background(), "s", map[string]Question{ + "needs_human_decision": Noul("a statement", "", ""), + "blocked": Noul("another statement", "", ""), + }) + if err == nil { + t.Fatal("Ask succeeded with one of two questions unanswered") + } + if !strings.Contains(err.Error(), "blocked") { + t.Errorf("error does not name the unanswered question: %v", err) + } + // The KIND check would also refuse this call — a missing answer decodes + // with an empty Kind — and would also name the key, so the two + // assertions above cannot tell which check fired. The diagnosis is the + // difference a caller sees: "no answer" versus "answered with a ", + // the second naming a kind the provider never sent. + if !strings.Contains(err.Error(), "no answer") { + t.Errorf("error does not say the question went unanswered: %v", err) + } +} + +// --------------------------------------------------------------------------- +// Truncation +// --------------------------------------------------------------------------- + +func TestOversizedStringStateIsTruncatedAndFlagged(t *testing.T) { + p, f := newFake(t, ok(recordedNoulResponse)) + + // Multi-byte runes so a naive byte cut would produce invalid UTF-8. + big := strings.Repeat("état ", 40000) + if len(big) <= int(float64(maxRequestTokens)*charsPerToken) { + t.Fatalf("test state is not actually over budget (%d chars)", len(big)) + } + + _, usage, err := p.Ask(context.Background(), big, map[string]Question{ + "needs_human_decision": Noul("a statement", "", ""), + }) + if err != nil { + t.Fatalf("Ask: %v", err) + } + if !usage.StateTruncated { + t.Error("StateTruncated is false after an over-budget state was shortened") + } + + sent, okStr := f.requests[0].State.(string) + if !okStr { + t.Fatalf("state arrived as %T, want string", f.requests[0].State) + } + if len(sent) >= len(big) { + t.Errorf("state was not shortened: sent %d chars of %d", len(sent), len(big)) + } + if !strings.HasPrefix(big, sent) { + t.Error("truncated state is not a prefix of the original") + } + // The cut must leave room for the questions too, since the measured + // budget covers state AND questions together. + if len(sent) > int(float64(maxRequestTokens)*charsPerToken) { + t.Errorf("truncated state (%d chars) exceeds the whole-request char budget", len(sent)) + } +} + +// TestBudgetConstantsStayInsideTheMeasurement guards the two constants that no +// behavioural test can check, because their correctness is a fact about the +// LIVE provider rather than about this code. Without this, someone raising +// either one to "use more of the window" gets a fully green suite and a +// production 400. +// +// Both figures come from the bisection recorded on TASK-3116's trail. +func TestBudgetConstantsStayInsideTheMeasurement(t *testing.T) { + // Largest request the live API ACCEPTED, by its own usage.input_tokens. + const measuredAccepted = 32476 + // Smallest request it REFUSED with max_tokens_exceeded (predicted from + // the calibrated tokens-per-record; the refusal itself is measured). + const measuredRefused = 33600 + + if maxRequestTokens > measuredAccepted { + t.Errorf("maxRequestTokens = %d, which is above the largest request measured to be "+ + "ACCEPTED (%d). Raising it past the measurement needs a new bisection, not a guess.", + maxRequestTokens, measuredAccepted) + } + if maxRequestTokens >= measuredRefused { + t.Errorf("maxRequestTokens = %d, at or above a request measured to be REFUSED (%d)", + maxRequestTokens, measuredRefused) + } + + // charsPerToken must UNDER-estimate characters per token so that the + // token estimate over-estimates and truncation stays conservative. The + // bisection measured 2.80 chars/token on the densest content tried. + const measuredCharsPerToken = 2.80 + if charsPerToken > measuredCharsPerToken { + t.Errorf("charsPerToken = %v, above the measured %v: truncation would under-estimate "+ + "tokens and send over-budget requests", charsPerToken, measuredCharsPerToken) + } +} + +// TestFitStateCutsOnRuneBoundaries sweeps the cut OFFSET, which is the whole +// point: the offset is a function of how large the questions are, so a single +// fixed state and question produce a single fixed offset. An earlier version of +// this assertion lived inside the truncation test above with one offset, and a +// mutant that deleted the rune-boundary loop entirely SURVIVED it — that one +// offset happened to land on a boundary, so the green could not go red. +// +// Varying the instructions length by one byte at a time shifts the offset by +// one, so across eight iterations over two-byte runes the cut lands mid-rune +// several times. The mutant dies on those. +func TestFitStateCutsOnRuneBoundaries(t *testing.T) { + p := newTypesafe("test-key", "jev-1.13.0") + + // Why 64 offsets and three rune widths, rather than a handful: a cut + // that splits a rune leaves a lone lead byte, which json.Marshal + // replaces with a six-byte \ufffd — so a mid-rune cut often FAILS the + // size check on its own and the loop retries, landing wherever the + // retry's arithmetic puts it. That second mechanism hides the defect at + // most offsets. Measured with the boundary loop deleted (review round 2): + // invalid UTF-8 escaped at 18/64 offsets for 2-byte runes, 29/64 for + // 3-byte and 43/64 for 4-byte, while the previous 8-offset sweep over + // 2-byte runes caught NONE. + sawTruncation := false + for _, r := range []string{"é", "€", "😀"} { + big := strings.Repeat(r, 400000) + for extra := 0; extra < 64; extra++ { + questions := map[string]wireQuestion{ + "q": {Type: "noul", Instructions: "i" + strings.Repeat("x", extra)}, + } + got, truncated, err := p.fitState(big, questions) + if err != nil { + t.Fatalf("%q extra=%d: fitState: %v", r, extra, err) + } + if !truncated { + t.Fatalf("%q extra=%d: state of %d bytes was not truncated", r, extra, len(big)) + } + sawTruncation = true + + s, okStr := got.(string) + if !okStr { + t.Fatalf("%q extra=%d: fitState returned %T, want string", r, extra, got) + } + if !utf8.ValidString(s) { + t.Errorf("%q extra=%d: truncated state is not valid UTF-8 — the cut split a rune "+ + "at offset %d", r, extra, len(s)) + } + if !strings.HasPrefix(big, s) { + t.Errorf("%q extra=%d: truncated state is not a prefix of the original", r, extra) + } + } + } + // Precondition: if nothing truncated, every assertion above was vacuous. + if !sawTruncation { + t.Fatal("no iteration truncated; the test asserted nothing") + } +} + +func TestShortStateIsNotTruncated(t *testing.T) { + p, f := newFake(t, ok(recordedNoulResponse)) + _, usage, err := p.Ask(context.Background(), "a short body", map[string]Question{ + "needs_human_decision": Noul("a statement", "", ""), + }) + if err != nil { + t.Fatalf("Ask: %v", err) + } + if usage.StateTruncated { + t.Error("StateTruncated is true for a state well inside the budget") + } + if got := f.requests[0].State; got != "a short body" { + t.Errorf("state = %v, want it passed through verbatim", got) + } +} + +func TestStructuredStateIsNeverTruncated(t *testing.T) { + // Cutting marshalled JSON at a byte boundary produces invalid JSON, and + // dropping fields would change what is being asked about. A structured + // state that does not fit gets the provider's own refusal instead. + p, f := newFake(t, ok(recordedNoulResponse)) + state := map[string]any{"body": strings.Repeat("x", 200000)} + + _, usage, err := p.Ask(context.Background(), state, map[string]Question{ + "needs_human_decision": Noul("a statement", "", ""), + }) + if err != nil { + t.Fatalf("Ask: %v", err) + } + if usage.StateTruncated { + t.Error("StateTruncated is true for a structured state, which is never cut") + } + m, okMap := f.requests[0].State.(map[string]any) + if !okMap { + t.Fatalf("state arrived as %T, want a JSON object", f.requests[0].State) + } + if s, _ := m["body"].(string); len(s) != 200000 { + t.Errorf("structured state body is %d chars, want the full 200000 — it must not be cut", len(s)) + } +} + +// --------------------------------------------------------------------------- +// Question validation +// --------------------------------------------------------------------------- + +func TestQuestionValidation(t *testing.T) { + tests := []struct { + name string + q Question + wantErr string + }{ + {"choice ok", Choice("i", map[string]string{"a": "x", "b": "y"}), ""}, + {"score ok", Score("i", []string{"a", "b"}), ""}, + {"noul ok with criteria", Noul("i", "t", "f"), ""}, + {"noul ok bare", Noul("i", "", ""), ""}, + + {"empty instructions", Choice(" ", map[string]string{"a": "x", "b": "y"}), "instructions are empty"}, + {"choice one option", Choice("i", map[string]string{"a": "x"}), "at least 2 options"}, + {"choice empty option name", Choice("i", map[string]string{"": "x", "b": "y"}), "empty option name"}, + {"choice empty description", Choice("i", map[string]string{"a": " ", "b": "y"}), "empty description"}, + {"score one level", Score("i", []string{"a"}), "at least 2 levels"}, + {"score empty level", Score("i", []string{"a", " "}), "level 1 is empty"}, + {"unknown kind", Question{Kind: "guess", Instructions: "i"}, "unknown question kind"}, + + // Cross-kind criteria. Each of these is a caller who meant a + // different constructor; silently dropping the stray field would + // send a question they did not write. + {"choice with levels", Question{Kind: KindChoice, Instructions: "i", + Options: map[string]string{"a": "x", "b": "y"}, Levels: []string{"l1", "l2"}}, + "another kind's criteria"}, + {"score with options", Question{Kind: KindScore, Instructions: "i", + Levels: []string{"a", "b"}, Options: map[string]string{"o": "d"}}, + "another kind's criteria"}, + {"noul with options", Question{Kind: KindNoul, Instructions: "i", + Options: map[string]string{"a": "x"}}, "another kind's criteria"}, + {"noul half-described", Question{Kind: KindNoul, Instructions: "i", TrueDesc: "t"}, + "only one of true/false"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.q.Validate() + if tt.wantErr == "" { + if err != nil { + t.Fatalf("Validate() = %v, want nil", err) + } + return + } + if err == nil { + t.Fatalf("Validate() = nil, want an error containing %q", tt.wantErr) + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Errorf("Validate() = %q, want it to contain %q", err.Error(), tt.wantErr) + } + }) + } +} + +func TestAskRejectsBadInputBeforeCallingProvider(t *testing.T) { + called := false + p, _ := newFake(t, func(_ *fake, _ wireRequest, _ []byte, w http.ResponseWriter) { + called = true + _, _ = w.Write([]byte(recordedNoulResponse)) + }) + + if _, _, err := p.Ask(context.Background(), "s", nil); err == nil { + t.Error("Ask succeeded with no questions") + } + if _, _, err := p.Ask(context.Background(), "s", map[string]Question{"": Noul("i", "", "")}); err == nil { + t.Error("Ask succeeded with an empty question key") + } + if _, _, err := p.Ask(context.Background(), "s", map[string]Question{ + "q": Choice("i", map[string]string{"only": "one"}), + }); err == nil { + t.Error("Ask succeeded with an invalid choice question") + } else if !strings.Contains(err.Error(), `question "q"`) { + t.Errorf("error does not name the offending question key: %v", err) + } + + // The whole point of validating first: no request was ever sent. + if called { + t.Error("provider was called despite invalid input") + } +} diff --git a/internal/decision/live_test.go b/internal/decision/live_test.go new file mode 100644 index 000000000..d664a905e --- /dev/null +++ b/internal/decision/live_test.go @@ -0,0 +1,146 @@ +package decision + +import ( + "context" + "os" + "testing" + "time" +) + +// TestLiveProviderAllThreePrimitives calls the REAL provider. +// +// It is skipped unless DECISION_LIVE=1 and PAD_TYPESAFE_API_KEY are both set, +// so CI and `make test` never reach it — it costs money and needs network. +// +// It exists because the httptest fake cannot answer the question this does. +// The fake replays bodies I wrote and asserts against shapes I chose, so an +// encoder and a fake that share the same wrong field name agree with each +// other and both go green: the instrument's subject controls what the +// instrument reads. This is the second channel. Run it after any change to the +// wire types: +// +// set -a && . ./.env && set +a && DECISION_LIVE=1 go test ./internal/decision/ -run Live -v +func TestLiveProviderAllThreePrimitives(t *testing.T) { + if os.Getenv("DECISION_LIVE") != "1" { + t.Skip("set DECISION_LIVE=1 to call the real provider") + } + key := os.Getenv("PAD_TYPESAFE_API_KEY") + if key == "" { + // Accept the vendor's own variable name too, since that is what the + // eval scripts and .env use. + key = os.Getenv("TYPESAFE_API_KEY") + } + if key == "" { + t.Skip("no API key in the environment") + } + + p, err := New(Config{Provider: ProviderTypesafe, APIKey: key}) + if err != nil { + t.Fatalf("New: %v", err) + } + + ctx, cancel := context.WithTimeout(context.Background(), 90*time.Second) + defer cancel() + + state := "A bug report: the mermaid diagram renderer uses a hardcoded dark theme " + + "even when the application is in light mode. Cosmetic, affects every diagram, " + + "no data is wrong and no flow is blocked. Nobody is waiting on a decision." + + answers, usage, err := p.Ask(ctx, state, map[string]Question{ + "component": Choice("Which part of the codebase does the fix live in?", map[string]string{ + "web": "the SvelteKit web UI", + "server": "the Go HTTP API handlers", + "store": "the database layer and migrations", + }), + "severity": Score("Rate how severe this bug is for users.", []string{ + "cosmetic only", "annoying but harmless", "a real defect in normal use", + "breaks a core flow", "data loss or security exposure", + }), + "needs_human": Noul("The next step requires a judgment only a human can supply.", + "a human must decide", "an agent can proceed"), + }) + if err != nil { + t.Fatalf("Ask against the live provider: %v", err) + } + t.Logf("usage: %+v", usage) + + // Choice: an option name we sent, with a confidence and name-keyed + // probabilities. A wrong field name shows up here as a zero value. + c := answers["component"] + if c.Kind != KindChoice { + t.Errorf("component Kind = %q, want choice", c.Kind) + } + if c.Choice != "web" && c.Choice != "server" && c.Choice != "store" { + t.Errorf("component Choice = %q, want one of the options we sent", c.Choice) + } + if c.Confidence == nil { + t.Error("live choice answer decoded no confidence — the field name is wrong") + } + if len(c.Probabilities) != 3 { + t.Errorf("component Probabilities = %v, want one entry per option sent", c.Probabilities) + } + if _, ok := c.Probabilities[c.Choice]; !ok { + t.Errorf("choice %q is not a key of its own probabilities %v — choice probabilities "+ + "must be keyed by option NAME", c.Choice, c.Probabilities) + } + t.Logf("choice: %q conf=%v probs=%v", c.Choice, *c.Confidence, c.Probabilities) + + // Score: index-keyed probabilities, a legend echoing our levels, and the + // mean-not-index property re-verified against live numbers. + s := answers["severity"] + if s.Kind != KindScore { + t.Errorf("severity Kind = %q, want score", s.Kind) + } + if len(s.Legend) != 5 { + t.Errorf("severity Legend = %v, want 5 entries echoing the levels sent", s.Legend) + } + if s.Legend["0"] != "cosmetic only" { + t.Errorf("Legend[0] = %q, want the first level we sent", s.Legend["0"]) + } + if s.Confidence == nil { + t.Error("live score answer decoded no confidence — the field name is wrong") + } + var mean float64 + for k, prob := range s.Probabilities { + mean += float64(levelIndex(k)) * prob + } + if diff := mean - s.Score; diff > 0.02 || diff < -0.02 { + t.Errorf("Score %v is not the probability-weighted mean of the level indices (%v). "+ + "The doc comment on Answer.Score asserts it is; if this fails, that comment is "+ + "now false and Answer.TopLevel's contract needs re-reading.", s.Score, mean) + } + idx, name, okTop := s.TopLevel() + if !okTop { + t.Error("TopLevel() reported no level for a live score answer") + } + t.Logf("score: %v conf=%v top=(%d,%q) probs=%v legend-size=%d", + s.Score, *s.Confidence, idx, name, s.Probabilities, len(s.Legend)) + + // Noul: a probability, and NO confidence. This is the assertion the whole + // pointer design rests on, verified against the live provider rather than + // against a recording of it. + n := answers["needs_human"] + if n.Kind != KindNoul { + t.Errorf("needs_human Kind = %q, want noul", n.Kind) + } + if n.Noul < 0 || n.Noul > 1 { + t.Errorf("needs_human Noul = %v, want a probability in [0,1]", n.Noul) + } + if n.Confidence != nil { + t.Errorf("LIVE noul answer carried a confidence (%v). Answer.Confidence is a pointer "+ + "precisely because it does not; if the provider has started sending one, the "+ + "doc comments and the recorded fixtures need updating.", *n.Confidence) + } + if n.Probabilities != nil { + t.Errorf("live noul answer carried probabilities %v, want none", n.Probabilities) + } + t.Logf("noul: %v (confidence absent as expected)", n.Noul) + + if usage.InputTokens <= 0 { + t.Errorf("usage.InputTokens = %d, want a positive count — the field name may be wrong", + usage.InputTokens) + } + if usage.StateTruncated { + t.Error("a short live state was reported as truncated") + } +} diff --git a/internal/decision/review2_test.go b/internal/decision/review2_test.go new file mode 100644 index 000000000..cd6602eca --- /dev/null +++ b/internal/decision/review2_test.go @@ -0,0 +1,236 @@ +package decision + +import ( + "context" + "encoding/json" + "errors" + "io" + "net/http" + "strings" + "testing" + "time" +) + +// Review round 2 (TASK-3116). Round 1 added the KIND check; round 2 found the +// rest of that population: an answer can contradict its question in every +// (kind, member) cell, not only in its kind. The rows below are that table — +// one refusal per rule checkAnswer enforces, and an ACCEPTANCE row at each +// boundary, so a check made too strict fails here as surely as one removed. + +var ( + choiceQ = Choice("How severe is this bug?", map[string]string{ + "low": "cosmetic", "medium": "a real defect", "high": "breaks a core flow", "critical": "data loss", + }) + scoreQ = Score("How severe is this bug?", []string{ + "cosmetic only", "annoying but harmless", "a real defect in normal use", "breaks a core flow", "data loss or security exposure", + }) + noulQ = Noul("a statement", "", "") +) + +func TestAnswerContradictingTheQuestionIsRefused(t *testing.T) { + const legend5 = `"legend":{"0":"a","1":"b","2":"c","3":"d","4":"e"}` + tests := []struct { + name string + q Question + answer string // the answer object for key "q" + wantErr string // "" means the answer must be ACCEPTED + }{ + // choice + {"choice accepted", choiceQ, `{"type":"choice","choice":"high","confidence":0.63,"probabilities":{"high":0.72,"low":0.28}}`, ""}, + {"choice confidence 0 and 1 accepted", choiceQ, `{"type":"choice","choice":"low","confidence":0,"probabilities":{"low":1,"high":0}}`, ""}, + {"choice missing", choiceQ, `{"type":"choice","confidence":0.6,"probabilities":{"high":1}}`, "carries no choice"}, + {"choice not among options", choiceQ, `{"type":"choice","choice":"urgent","confidence":0.6,"probabilities":{"high":1}}`, "not one of the options sent"}, + {"choice missing confidence", choiceQ, `{"type":"choice","choice":"high","probabilities":{"high":1}}`, "carries no confidence"}, + {"choice confidence above 1", choiceQ, `{"type":"choice","choice":"high","confidence":1.5,"probabilities":{"high":1}}`, "confidence 1.5 is outside"}, + {"choice confidence below 0", choiceQ, `{"type":"choice","choice":"high","confidence":-0.1,"probabilities":{"high":1}}`, "confidence -0.1 is outside"}, + {"choice missing probabilities", choiceQ, `{"type":"choice","choice":"high","confidence":0.6}`, "carries no probabilities"}, + {"choice probability key not an option", choiceQ, `{"type":"choice","choice":"high","confidence":0.6,"probabilities":{"high":0.5,"urgent":0.5}}`, `key "urgent"`}, + {"choice probability above 1", choiceQ, `{"type":"choice","choice":"high","confidence":0.6,"probabilities":{"high":1.2}}`, "probability 1.2"}, + + // score + {"score accepted", scoreQ, `{"type":"score","score":0.14,"confidence":0.88,` + legend5 + `,"probabilities":{"0":0.86,"1":0.14}}`, ""}, + {"score at the top level accepted", scoreQ, `{"type":"score","score":4,"confidence":1,` + legend5 + `,"probabilities":{"4":1}}`, ""}, + {"score at zero accepted", scoreQ, `{"type":"score","score":0,"confidence":1,"probabilities":{"0":1}}`, ""}, + {"score missing", scoreQ, `{"type":"score","confidence":0.8,"probabilities":{"0":1}}`, "carries no score"}, + {"score above the top level", scoreQ, `{"type":"score","score":4.5,"confidence":0.8,"probabilities":{"4":1}}`, "outside [0, 4]"}, + {"score below zero", scoreQ, `{"type":"score","score":-0.1,"confidence":0.8,"probabilities":{"0":1}}`, "outside [0, 4]"}, + {"score missing confidence", scoreQ, `{"type":"score","score":1,"probabilities":{"1":1}}`, "carries no confidence"}, + {"score missing probabilities", scoreQ, `{"type":"score","score":1,"confidence":0.8}`, "carries no probabilities"}, + {"score probability key past the levels", scoreQ, `{"type":"score","score":1,"confidence":0.8,"probabilities":{"5":1}}`, `key "5"`}, + {"score probability key non-canonical", scoreQ, `{"type":"score","score":1,"confidence":0.8,"probabilities":{"01":1}}`, `key "01"`}, + {"score probability key not a number", scoreQ, `{"type":"score","score":1,"confidence":0.8,"probabilities":{"junk":1}}`, `key "junk"`}, + {"score probability key negative", scoreQ, `{"type":"score","score":1,"confidence":0.8,"probabilities":{"-1":1}}`, `key "-1"`}, + {"score legend key past the levels", scoreQ, `{"type":"score","score":1,"confidence":0.8,"legend":{"9":"x"},"probabilities":{"1":1}}`, `legend key "9"`}, + {"score probability below 0", scoreQ, `{"type":"score","score":1,"confidence":0.8,"probabilities":{"1":-0.2}}`, "probability -0.2"}, + + // noul + {"noul accepted", noulQ, `{"type":"noul","noul":0.92}`, ""}, + {"noul at 1 accepted", noulQ, `{"type":"noul","noul":1}`, ""}, + {"noul zero accepted", noulQ, `{"type":"noul","noul":0}`, ""}, + {"noul missing", noulQ, `{"type":"noul"}`, "carries no noul value"}, + {"noul above 1", noulQ, `{"type":"noul","noul":1.01}`, "noul 1.01 is outside"}, + {"noul below 0", noulQ, `{"type":"noul","noul":-0.01}`, "noul -0.01 is outside"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p, _ := newFake(t, ok(`{"model":"jev-1.13.0","answers":{"q":`+tt.answer+`},"usage":{"input_tokens":1,"output_tokens":1}}`)) + answers, _, err := p.Ask(context.Background(), "s", map[string]Question{"q": tt.q}) + if tt.wantErr == "" { + if err != nil { + t.Fatalf("a valid answer was refused: %v", err) + } + if answers["q"].Kind != tt.q.Kind { + t.Errorf("accepted answer has Kind %q, want %q", answers["q"].Kind, tt.q.Kind) + } + return + } + if err == nil { + t.Fatalf("Ask accepted an answer that contradicts its question; want an error containing %q", tt.wantErr) + } + if answers != nil { + t.Errorf("Ask returned answers alongside its error: %v — the call is all-or-nothing", answers) + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Errorf("error = %q, want it to contain %q", err.Error(), tt.wantErr) + } + if !strings.Contains(err.Error(), `question "q"`) { + t.Errorf("error does not name the question: %q", err.Error()) + } + }) + } +} + +func TestUnsolicitedAnswerIsDropped(t *testing.T) { + p, _ := newFake(t, ok(`{"model":"jev-1.13.0","answers":{"q":{"type":"noul","noul":0.3},"internal_debug":{"type":"noul","noul":0.9}},"usage":{"input_tokens":1,"output_tokens":1}}`)) + answers, _, err := p.Ask(context.Background(), "s", map[string]Question{"q": noulQ}) + if err != nil { + t.Fatalf("Ask: %v", err) + } + if _, ok := answers["internal_debug"]; ok { + t.Error("an answer to a question nobody asked was returned to the caller") + } + if len(answers) != 1 || answers["q"].Noul != 0.3 { + t.Errorf("answers = %v, want exactly the one asked-for answer", answers) + } +} + +// The envelope reservation used to be a fixed 512 bytes. A model name longer +// than that, plus a state cut exactly to the limit, sent a request over the +// budget. The reservation is now the marshalled envelope itself. +func TestFittedRequestStaysInsideTheBudgetWithALongModelName(t *testing.T) { + p := newTypesafe("k", strings.Repeat("m", 5000)) + wire := map[string]wireQuestion{"q": toWire(noulQ)} + budget := int(float64(maxRequestTokens) * charsPerToken) + + state, truncated, err := p.fitState(strings.Repeat("x", budget), wire) + if err != nil { + t.Fatalf("fitState: %v", err) + } + if !truncated { + t.Fatal("precondition: a budget-sized state must be truncated, or this test measures nothing") + } + full, err := json.Marshal(wireRequest{Model: p.model, State: state, Questions: wire}) + if err != nil { + t.Fatalf("marshal: %v", err) + } + if len(full) > budget { + t.Errorf("fitted request is %d bytes, over the %d-byte budget by %d", len(full), budget, len(full)-budget) + } + // And the cut is tight, not merely safe: the reservation is exact, so the + // whole request lands ON the budget. A reservation that over-reserved + // would still pass the check above. + if len(full) != budget { + t.Errorf("fitted request is %d bytes, want exactly the %d-byte budget for a one-byte-per-char state", len(full), budget) + } +} + +// Round 2: the partial-read failure sat on BodyReadErr but was invisible to +// errors.Is, so a caller could not ask "did this fail because the read was +// cut off" without knowing the field. +func TestPartialBodyReadIsReachableWithErrorsIs(t *testing.T) { + srv := newRawServer(t, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Length", "200") + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"detail":{"error_ty`)) + }) + p := newTypesafe("test-key", "jev-1.13.0") + p.setEndpoint(srv.URL) + p.sleep = func(context.Context, time.Duration) error { return nil } + + _, _, err := p.Ask(context.Background(), "s", map[string]Question{"q": noulQ}) + var apiErr *APIError + if !errors.As(err, &apiErr) || apiErr.BodyReadErr == nil { + t.Fatalf("precondition: want an *APIError carrying a BodyReadErr, got %v", err) + } + if !errors.Is(err, apiErr.BodyReadErr) { + t.Errorf("errors.Is(err, BodyReadErr) is false; the read failure is not in the chain") + } + if !errors.Is(err, io.ErrUnexpectedEOF) { + t.Errorf("errors.Is(err, io.ErrUnexpectedEOF) is false for a body cut short (BodyReadErr = %v)", apiErr.BodyReadErr) + } + // Control leg: a complete body carries no read error, so nothing + // spurious enters the chain. + p2, _ := newFake(t, func(_ *fake, _ wireRequest, _ []byte, w http.ResponseWriter) { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"detail":{"error_type":"api_usage_error","message":"Invalid request."}}`)) + }) + _, _, err = p2.Ask(context.Background(), "s", map[string]Question{"q": noulQ}) + if errors.Is(err, io.ErrUnexpectedEOF) { + t.Error("a complete error body reported an unexpected EOF") + } +} + +// Review round 3: with 0 or 1 bytes left for the state, fitState returned +// "" — which still marshals to two bytes, so the request went over budget. +// This walks `remaining` across the edge by growing the instructions a byte +// at a time and holds one invariant at every step: either an error, or a +// request that fits. +func TestFitStateHoldsTheBudgetAtTheQuoteEdge(t *testing.T) { + p := newTypesafe("test-key", "jev-1.13.0") + budget := int(float64(maxRequestTokens) * charsPerToken) + seen := map[int]bool{} + + for n := budget - 120; n < budget; n++ { + wire := map[string]wireQuestion{"q": {Type: "noul", Instructions: strings.Repeat("i", n)}} + env, err := json.Marshal(wireRequest{Model: p.model, State: "", Questions: wire}) + if err != nil { + t.Fatal(err) + } + remaining := budget - (len(env) - 2) + if remaining < -2 || remaining > 5 { + continue + } + seen[remaining] = true + // A structured state is never cut, and `0` marshals to ONE byte, so + // with remaining >= 1 it fits and must not be refused (round 4). + if remaining >= 1 { + got, truncated, err := p.fitState(0, wire) + if err != nil { + t.Errorf("remaining=%d: a one-byte structured state was refused: %v", remaining, err) + } else if truncated || got != 0 { + t.Errorf("remaining=%d: structured state came back %v (truncated=%v), want it untouched", remaining, got, truncated) + } + } + for _, state := range []string{"", "abc", strings.Repeat("x", 50)} { + got, _, err := p.fitState(state, wire) + if err != nil { + if remaining >= 2 { + t.Errorf("remaining=%d state=%q: refused although a state fits: %v", remaining, state, err) + } + continue + } + full, _ := json.Marshal(wireRequest{Model: p.model, State: got, Questions: wire}) + if len(full) > budget { + t.Errorf("remaining=%d state=%q: request is %d bytes, %d over the budget", + remaining, state, len(full), len(full)-budget) + } + } + } + // Precondition: the sweep must actually have crossed the edge, or it + // asserted nothing about it. + for _, r := range []int{0, 1, 2} { + if !seen[r] { + t.Errorf("sweep never produced remaining=%d; widen it", r) + } + } +} diff --git a/internal/decision/review_test.go b/internal/decision/review_test.go new file mode 100644 index 000000000..8e78b0115 --- /dev/null +++ b/internal/decision/review_test.go @@ -0,0 +1,406 @@ +package decision + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + "unicode/utf8" +) + +// One test per finding from review round 1 (backend: opencode/gpt-5.6-luna). +// Each asserts the CONSEQUENCE the finding described, so it fails on the +// pre-fix code rather than merely exercising the new lines. + +// Finding 1 — the backoff ignored context cancellation, so a cancelled Ask +// waited out the full wait (up to maxBackoff) before issuing a doomed retry. +// +// The finding names ONE mechanism and it needs two instruments, because the +// obvious single test does not reach the code it claims to. Cancelling during +// the first response makes http.Client.Do fail, so Ask returns from the +// TRANSPORT-error branch having never reached the sleep at all — the first +// version of this test asserted one recorded wait and got zero, which is how +// the substitution showed up. So: this test pins that Ask HONOURS an +// interrupted wait, and TestCtxSleepReturnsEarlyOnCancellation pins that the +// wait actually installed on a real provider IS interruptible. Neither alone +// covers the finding. +func TestAskAbortsWhenTheBackoffIsInterrupted(t *testing.T) { + sentinel := errors.New("wait interrupted") + + calls := 0 + p, f := newFake(t, func(_ *fake, _ wireRequest, _ []byte, w http.ResponseWriter) { + calls++ + w.WriteHeader(http.StatusTooManyRequests) + }) + // Reach the sleep on a live context, then have the wait report that it was + // interrupted. This is what a cancellation landing inside the backoff + // looks like from Ask's point of view, without racing a real timer. + p.sleep = func(_ context.Context, d time.Duration) error { + f.slept = append(f.slept, d) + return sentinel + } + + _, _, err := p.Ask(context.Background(), "s", map[string]Question{"q": Noul("a statement", "", "")}) + if !errors.Is(err, sentinel) { + t.Fatalf("err = %v, want the interrupted wait's error to propagate", err) + } + // One request, one interrupted wait, and no further attempts: the loop + // must stop rather than proceed into a doomed retry. + if calls != 1 { + t.Errorf("provider was called %d times after the wait was interrupted, want 1", calls) + } + if len(f.slept) != 1 { + t.Errorf("slept %v, want exactly one wait before aborting", f.slept) + } +} + +// Finding 1, the in-flight case: a cancellation while the request is on the +// wire must also end the call, and must not be swallowed as a retryable +// transport error. +func TestCancellationDuringTheRequestEndsTheCall(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + + calls := 0 + p, _ := newFake(t, func(_ *fake, _ wireRequest, _ []byte, w http.ResponseWriter) { + calls++ + cancel() + w.WriteHeader(http.StatusTooManyRequests) + }) + + _, _, err := p.Ask(ctx, "s", map[string]Question{"q": Noul("a statement", "", "")}) + if err == nil { + t.Fatal("Ask succeeded after its context was cancelled") + } + if !errors.Is(err, context.Canceled) { + t.Errorf("err = %v, want it to wrap context.Canceled", err) + } + if calls != 1 { + t.Errorf("provider was called %d times, want 1: a cancelled context must not be "+ + "retried as a transient transport failure", calls) + } +} + +// Finding 1, the production path: the real sleep must return early. The fake +// above can only show that Ask HONOURS an aborting sleep; this shows that the +// sleep actually installed on a real provider aborts. +func TestCtxSleepReturnsEarlyOnCancellation(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + start := time.Now() + err := ctxSleep(ctx, 5*time.Second) + elapsed := time.Since(start) + + if !errors.Is(err, context.Canceled) { + t.Errorf("ctxSleep returned %v, want context.Canceled", err) + } + if elapsed > time.Second { + t.Errorf("ctxSleep waited %v on an already-cancelled context, want it to return immediately", elapsed) + } + + // Control leg: with a live context it really does wait, so the assertion + // above is not passing because ctxSleep never waits at all. + start = time.Now() + if err := ctxSleep(context.Background(), 40*time.Millisecond); err != nil { + t.Errorf("ctxSleep on a live context returned %v, want nil", err) + } + if elapsed = time.Since(start); elapsed < 30*time.Millisecond { + t.Errorf("ctxSleep returned after %v for a 40ms wait — it is not waiting at all", elapsed) + } + + // And newTypesafe must actually install it; a provider left with a + // non-cancellable sleep would pass every test above. + // + // The call runs under its OWN deadline: an uncancellable sleep of an hour + // would otherwise hang until go test's global timeout, which reports a + // panic for the whole binary rather than a failure of this test. + p := newTypesafe("k", "m") + if p.sleep == nil { + t.Fatal("newTypesafe installed no sleep function") + } + done := make(chan error, 1) + go func() { done <- p.sleep(ctx, time.Hour) }() + select { + case err := <-done: + if !errors.Is(err, context.Canceled) { + t.Errorf("the sleep installed by newTypesafe returned %v on a cancelled context, "+ + "want context.Canceled — it is not cancellable", err) + } + case <-time.After(2 * time.Second): + t.Error("the sleep installed by newTypesafe was still waiting 2s after being handed " + + "a cancelled context — it is not cancellable") + } +} + +// Finding 2 — a body read that failed partway was discarded, so a caller could +// not tell an absent ErrorType from an unread one. +func TestPartialBodyReadIsCarriedOnTheError(t *testing.T) { + // Content-Length promises more than the handler writes, so the client's + // read fails partway with an unexpected EOF. + srv := newRawServer(t, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Length", "200") + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"detail":{"error_ty`)) + }) + p := newTypesafe("test-key", "jev-1.13.0") + p.setEndpoint(srv.URL) + p.sleep = func(context.Context, time.Duration) error { return nil } + + _, _, err := p.Ask(context.Background(), "s", map[string]Question{"q": Noul("a statement", "", "")}) + if err == nil { + t.Fatal("Ask succeeded on a 400 with a truncated body") + } + var apiErr *APIError + if !errors.As(err, &apiErr) { + t.Fatalf("err is not an *APIError: %v", err) + } + if apiErr.BodyReadErr == nil { + t.Error("BodyReadErr is nil after a body read that failed partway; a caller cannot " + + "tell the missing ErrorType from an unread one") + } + if !strings.Contains(err.Error(), "read failed partway") { + t.Errorf("Error() does not mention the partial read: %q", err.Error()) + } + // The partial bytes are still the best evidence available, so they are kept. + if !strings.Contains(apiErr.Body, "error_ty") { + t.Errorf("Body = %q, want the partial bytes that did arrive", apiErr.Body) + } +} + +// Finding 3 — exhausting retries wrapped a *transientError, which is not an +// *APIError, so errors.As could not reach the status or error type. +func TestExhaustedRetriesStillExposeTheAPIError(t *testing.T) { + p, _ := newFake(t, func(_ *fake, _ wireRequest, _ []byte, w http.ResponseWriter) { + w.WriteHeader(http.StatusTooManyRequests) + _, _ = w.Write([]byte(`{"detail":{"error_type":"rate_limited"}}`)) + }) + + _, _, err := p.Ask(context.Background(), "s", map[string]Question{"q": Noul("a statement", "", "")}) + if err == nil { + t.Fatal("Ask succeeded against a provider that only 429s") + } + + var apiErr *APIError + if !errors.As(err, &apiErr) { + t.Fatalf("errors.As(err, **APIError) = false after exhausted retries; the status and "+ + "error type are in the chain and unreachable. err = %v", err) + } + if apiErr.StatusCode != http.StatusTooManyRequests { + t.Errorf("StatusCode = %d, want 429", apiErr.StatusCode) + } + if apiErr.ErrorType != "rate_limited" { + t.Errorf("ErrorType = %q, want rate_limited", apiErr.ErrorType) + } + // The wrapper's own message must survive too — the caller needs to know + // it was a retry wall, not a single refusal. + if !strings.Contains(err.Error(), "after") { + t.Errorf("Error() does not say the retries were exhausted: %q", err.Error()) + } +} + +// Finding 4 — the truncation budget was measured on RAW string length while +// encoding/json escapes <, > and & to six-byte sequences, so a state of '<' +// characters passed the budget check and then drew max_tokens_exceeded. +func TestTruncationAccountsForJSONEscapeExpansion(t *testing.T) { + // Establish the premise rather than asserting it from memory: the + // expansion is real and it is 6x. + probe, err := json.Marshal(strings.Repeat("<", 1000)) + if err != nil { + t.Fatalf("marshal probe: %v", err) + } + if len(probe) < 6000 { + t.Fatalf("premise failed: 1000 '<' marshalled to %d bytes, expected ~6002 — "+ + "if encoding/json stopped escaping, this test and the fitState comment are stale", + len(probe)) + } + + p := newTypesafe("test-key", "jev-1.13.0") + questions := map[string]wireQuestion{"q": {Type: "noul", Instructions: "i"}} + budget := int(float64(maxRequestTokens) * charsPerToken) + + for _, tc := range []struct { + name string + state string + }{ + // Raw length just inside the char budget, marshalled length 6x over: + // this is the exact case the old code let through untouched. + {"all escaped", strings.Repeat("<", budget-1000)}, + {"mostly escaped", strings.Repeat(" budget { + t.Errorf("truncated state marshals to %d bytes, over the %d-byte budget — "+ + "the cut was sized on raw length", ml, budget) + } + if !utf8.ValidString(s) { + t.Error("truncated state is not valid UTF-8") + } + if !strings.HasPrefix(tc.state, s) { + t.Error("truncated state is not a prefix of the original") + } + // It must not over-truncate to nothing when something fits. + if len(s) == 0 { + t.Error("everything was cut; a state this size has room for some prefix") + } + }) + } +} + +// Finding 4, boundary: when the questions alone consume the budget there is no +// room for any state, and that must be an error or an empty state — never a +// panic or a negative slice index. +func TestFitStateWithNoRoomForState(t *testing.T) { + p := newTypesafe("test-key", "jev-1.13.0") + budget := int(float64(maxRequestTokens) * charsPerToken) + + huge := map[string]wireQuestion{"q": {Type: "noul", Instructions: strings.Repeat("i", budget*2)}} + if _, _, err := p.fitState("some state", huge); err == nil { + t.Error("fitState succeeded with questions larger than the whole budget, want an error") + } + + // Just-barely-no-room: the questions fit, but leave almost nothing. + tight := map[string]wireQuestion{"q": {Type: "noul", Instructions: strings.Repeat("i", budget-600)}} + got, _, err := p.fitState(strings.Repeat("<", 5000), tight) + if err != nil { + t.Fatalf("fitState: %v", err) + } + s, _ := got.(string) + if ml := marshalledLen(s); ml > budget { + t.Errorf("truncated state marshals to %d, over budget %d", ml, budget) + } + if !utf8.ValidString(s) { + t.Error("truncated state is not valid UTF-8") + } +} + +// Finding 4, the helper's own edge case I found while fixing it: an index at or +// past len(s) must not index out of range. +func TestRuneBoundaryAtOrBeforeEdges(t *testing.T) { + const s = "aé" // 3 bytes: 'a', then a two-byte rune + for _, tc := range []struct{ in, want int }{ + {-5, 0}, {0, 0}, {1, 1}, {2, 1}, {3, 3}, {4, 3}, {100, 3}, + } { + if got := runeBoundaryAtOrBefore(s, tc.in); got != tc.want { + t.Errorf("runeBoundaryAtOrBefore(%q, %d) = %d, want %d", s, tc.in, got, tc.want) + } + } + if got := runeBoundaryAtOrBefore("", 0); got != 0 { + t.Errorf("runeBoundaryAtOrBefore(\"\", 0) = %d, want 0", got) + } +} + +// Finding 5 — an answer whose kind differs from its question's was accepted, +// and fromWire then left the requested kind's member at a plausible zero. +func TestAnswerKindMustMatchTheQuestion(t *testing.T) { + // A noul question answered with a score. Noul would read 0.0 — "almost + // certainly false" — which the provider never said. + p, _ := newFake(t, ok(`{"model":"jev-1.13.0","answers":{"needs_human":{"type":"score","score":0,"confidence":0.9,"probabilities":{"0":1}}},"usage":{"input_tokens":10,"output_tokens":2}}`)) + + _, _, err := p.Ask(context.Background(), "s", map[string]Question{ + "needs_human": Noul("a statement", "", ""), + }) + if err == nil { + t.Fatal("Ask accepted a score answer to a noul question; the caller would have read " + + "Noul == 0.0, a confident 'false' the provider never said") + } + if !strings.Contains(err.Error(), "needs_human") { + t.Errorf("error does not name the question: %v", err) + } + if !strings.Contains(err.Error(), string(KindNoul)) || !strings.Contains(err.Error(), string(KindScore)) { + t.Errorf("error does not name both the asked and answered kinds: %v", err) + } + + // Control leg: the matching kind still succeeds, so the check is not + // simply refusing everything. + p2, _ := newFake(t, ok(recordedNoulResponse)) + if _, _, err := p2.Ask(context.Background(), "s", map[string]Question{ + "needs_human_decision": Noul("a statement", "", ""), + }); err != nil { + t.Errorf("a correctly-typed answer was refused: %v", err) + } +} + +// Finding 6 — TopLevel ordered only on the parsed index, so distinct keys +// sharing an index ("1" and "01"), or several unparseable keys sharing the +// sentinel, were tie-broken by Go's randomised map iteration order. +func TestTopLevelIsDeterministicForDegenerateKeys(t *testing.T) { + cases := []struct { + name string + answer Answer + wantName string + }{ + { + // Same numeric index, different key strings, equal probability. + name: "numerically duplicate keys", + answer: Answer{Kind: KindScore, + Legend: map[string]string{"1": "one", "01": "oh-one"}, + Probabilities: map[string]float64{"1": 0.5, "01": 0.5}}, + wantName: "oh-one", // key "01" sorts before "1" + }, + { + // Several unparseable keys all collapse to the same sentinel. + name: "unparseable keys", + answer: Answer{Kind: KindScore, + Legend: map[string]string{"zz": "zed", "aa": "ay", "mm": "em"}, + Probabilities: map[string]float64{"zz": 0.4, "aa": 0.4, "mm": 0.4}}, + wantName: "ay", // "aa" sorts first among the tied sentinels + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + // Repeat: a map-order-dependent implementation passes + // intermittently, so a single iteration discriminates nothing. + for i := 0; i < 200; i++ { + _, name, okTop := tc.answer.TopLevel() + if !okTop { + t.Fatalf("iteration %d: TopLevel reported not-ok", i) + } + if name != tc.wantName { + t.Fatalf("iteration %d: TopLevel name = %q, want a stable %q — the tie-break "+ + "is decided by map iteration order", i, name, tc.wantName) + } + } + }) + } + + // A real index still beats an unparseable key, so the sentinel does not + // accidentally win by sorting first. + mixed := Answer{Kind: KindScore, + Legend: map[string]string{"0": "zero", "junk": "junk"}, + Probabilities: map[string]float64{"0": 0.5, "junk": 0.5}} + for i := 0; i < 50; i++ { + if idx, name, _ := mixed.TopLevel(); idx != 0 || name != "zero" { + t.Fatalf("iteration %d: TopLevel = (%d,%q), want the real index (0,\"zero\") to win "+ + "the tie against an unparseable key", i, idx, name) + } + } +} + +// newRawServer is for the cases needing control of the raw response — here, a +// Content-Length that lies — which the shared fake cannot express. +func newRawServer(t *testing.T, h http.HandlerFunc) *httptest.Server { + t.Helper() + s := httptest.NewServer(h) + t.Cleanup(s.Close) + return s +} diff --git a/internal/decision/typesafe.go b/internal/decision/typesafe.go new file mode 100644 index 000000000..7d3635fbd --- /dev/null +++ b/internal/decision/typesafe.go @@ -0,0 +1,699 @@ +package decision + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "strconv" + "strings" + "time" +) + +// defaultTypesafeEndpoint is typesafe.ai's System One endpoint. +const defaultTypesafeEndpoint = "https://api.typesafe.ai/v1/systemone" + +// Token budget for one request. +// +// maxRequestTokens is MEASURED against jev-1.13.0, not taken from the vendor +// docs, which state no limit. Bisecting on the API's own +// usage.input_tokens: 32,476 tokens was ACCEPTED and the next step up +// (~33,600) was refused with HTTP 400 max_tokens_exceeded. The budget counts +// state AND questions together — input_tokens includes the questions — so a +// large question set leaves less room for state. 32,000 sits under the +// measured accepted figure with ~476 tokens of headroom, and independently +// matches the Cloudflare model card's published context window, which is the +// only published number and is therefore unlikely to be reduced below this. +// Receipts, including the full bisection, are on TASK-3116's trail. +const maxRequestTokens = 32000 + +// charsPerToken converts characters to an estimated token count for +// truncation. +// +// It is a deliberate UNDER-estimate of characters per token, which makes the +// resulting token estimate an OVER-estimate and truncation conservative. The +// bisection above measured 2.80 chars/token on number-dense filler; ordinary +// English prose runs nearer 4. Dense content — base64, hashes, minified JSON — +// tokenizes worse than either, so this cannot be a guarantee: it is a cheap +// bound that keeps normal item content well inside the budget. The real +// backstop for pathological input is the provider's own refusal, surfaced as +// [ErrMaxTokensExceeded] rather than swallowed, so a caller can shrink its +// state instead of reading a silent truncation. +const charsPerToken = 2.5 + +// Retry policy for transient provider failures. +const ( + maxAttempts = 4 + initialBackoff = 500 * time.Millisecond + maxBackoff = 8 * time.Second + maxRetryAfter = 30 * time.Second + requestTimeout = 60 * time.Second + maxErrBodyLength = 2048 +) + +// ErrMaxTokensExceeded reports that the provider refused the request because +// state plus questions exceeded its token budget. +// +// It is distinguished from ordinary validation failures because the remedy is +// different and mechanical: send less state. The provider signals it with +// HTTP 400 and a machine-readable body ({"detail":{"error_type": +// "max_tokens_exceeded"}}), so this does not rest on matching prose. +var ErrMaxTokensExceeded = errors.New("decision: provider token budget exceeded") + +// APIError is an error response from the provider. It is returned directly +// for a non-retryable status, and is reachable with errors.As through the +// retry wrapper when a retryable status persists past the last attempt. +// +// The body is carried verbatim (bounded) because the provider's refusal +// message is the only way a caller can fix the call. It is also printed by +// Error(), which is safe only because the provider's refusal bodies do not +// echo the request: measured 2026-09-21 against jev-1.13.0 with canary +// strings in the state, the instructions and the criteria, three refusal +// shapes (unknown question type, unknown model, and the budget refusal) +// returned fixed {"detail":{"error_type","message"}} bodies naming none of +// them. Receipts on TASK-3116's trail. A refusal shape not in that set is +// unmeasured. +type APIError struct { + StatusCode int + // ErrorType is the provider's machine-readable code from + // detail.error_type, when present. + ErrorType string + // Body is the response body, truncated to a bounded length. + Body string + // BodyReadErr is set when reading the response body failed partway. + // Body then holds whatever arrived before the failure, and callers must + // not treat it as the provider's complete message — in particular a + // missing ErrorType may mean "could not read" rather than "not that + // error". Keeping the read failure instead of discarding it is the + // difference between a caller knowing the evidence is partial and + // silently trusting a truncated body. + BodyReadErr error +} + +func (e *APIError) Error() string { + var b strings.Builder + fmt.Fprintf(&b, "decision: provider returned %d", e.StatusCode) + if e.ErrorType != "" { + fmt.Fprintf(&b, " (%s)", e.ErrorType) + } + fmt.Fprintf(&b, ": %s", e.Body) + if e.BodyReadErr != nil { + fmt.Fprintf(&b, " [response body read failed partway: %v]", e.BodyReadErr) + } + return b.String() +} + +// Unwrap exposes a partial-read failure to errors.Is and errors.As, so a +// caller cancelling mid-read sees errors.Is(err, context.Canceled) rather +// than having to inspect BodyReadErr by name. +func (e *APIError) Unwrap() error { return e.BodyReadErr } + +// Is lets errors.Is(err, ErrMaxTokensExceeded) match the budget refusal +// without the caller inspecting the status code or the body. +func (e *APIError) Is(target error) bool { + return target == ErrMaxTokensExceeded && e.ErrorType == "max_tokens_exceeded" +} + +// typesafeProvider is the typesafe.ai Jev backend. +type typesafeProvider struct { + apiKey string + model string + endpoint string + client *http.Client + // sleep waits for d or until ctx is done, whichever comes first, + // returning ctx.Err() in the latter case. Tests replace it to record the + // requested durations without spending wall clock. + // + // It takes a context because a plain time.Sleep here made a cancelled + // Ask keep waiting out the full backoff — up to maxBackoff — before + // issuing a request that could only fail. Cancellation has to be able to + // interrupt the WAIT, not merely be noticed after it. + sleep func(ctx context.Context, d time.Duration) error +} + +func newTypesafe(apiKey, model string) *typesafeProvider { + return &typesafeProvider{ + apiKey: apiKey, + model: model, + endpoint: defaultTypesafeEndpoint, + client: &http.Client{Timeout: requestTimeout}, + sleep: ctxSleep, + } +} + +// ctxSleep waits for d, or returns early with ctx.Err() if the context is done +// first. +func ctxSleep(ctx context.Context, d time.Duration) error { + t := time.NewTimer(d) + defer t.Stop() + select { + case <-ctx.Done(): + return ctx.Err() + case <-t.C: + return nil + } +} + +func (p *typesafeProvider) Name() string { return ProviderTypesafe } +func (p *typesafeProvider) Model() string { return p.model } + +// setEndpoint points the provider at a different base URL. Intended for tests +// standing up an httptest server; production callers leave the default. +func (p *typesafeProvider) setEndpoint(u string) { p.endpoint = u } + +// wireQuestion is one question as the provider expects it. Criteria is +// `any` because its shape depends on the kind: a map for choice and noul, an +// ordered array for score. +type wireQuestion struct { + Type string `json:"type"` + Instructions string `json:"instructions"` + Criteria any `json:"criteria,omitempty"` +} + +type wireRequest struct { + Model string `json:"model"` + State any `json:"state"` + Questions map[string]wireQuestion `json:"questions"` +} + +// wireAnswer is one answer as the provider returns it. The value members are +// pointers so an ABSENT member is distinguishable from a zero one: a noul +// answer carries no confidence, and a response missing its `noul` would +// otherwise decode as 0.0, a confident "false" the provider never said. +// checkAnswer refuses any answer missing a member its kind requires. +type wireAnswer struct { + Type string `json:"type"` + Choice *string `json:"choice"` + Score *float64 `json:"score"` + Noul *float64 `json:"noul"` + Legend map[string]string `json:"legend"` + Probabilities map[string]float64 `json:"probabilities"` + Confidence *float64 `json:"confidence"` +} + +type wireUsage struct { + InputTokens int `json:"input_tokens"` + OutputTokens int `json:"output_tokens"` +} + +type wireResponse struct { + Model string `json:"model"` + Answers map[string]wireAnswer `json:"answers"` + Usage wireUsage `json:"usage"` +} + +// wireError is the provider's error envelope. detail is `any` because the +// provider returns an object for the cases measured here and the shape is +// undocumented; errorTypeOf digs the code out without depending on it. +type wireError struct { + Detail any `json:"detail"` +} + +// Ask implements [Provider]. +func (p *typesafeProvider) Ask(ctx context.Context, state any, questions map[string]Question) (map[string]Answer, Usage, error) { + if len(questions) == 0 { + return nil, Usage{}, errors.New("decision: no questions asked") + } + + wire := make(map[string]wireQuestion, len(questions)) + for key, q := range questions { + if strings.TrimSpace(key) == "" { + return nil, Usage{}, errors.New("decision: question key is empty") + } + if err := q.Validate(); err != nil { + return nil, Usage{}, fmt.Errorf("question %q: %w", key, err) + } + wire[key] = toWire(q) + } + + state, truncated, err := p.fitState(state, wire) + if err != nil { + return nil, Usage{}, err + } + + body, err := json.Marshal(wireRequest{Model: p.model, State: state, Questions: wire}) + if err != nil { + return nil, Usage{}, fmt.Errorf("decision: marshal request: %w", err) + } + + resp, err := p.post(ctx, body) + if err != nil { + return nil, Usage{}, err + } + + // Every answer is checked against the question it claims to answer + // before any of it reaches the caller: a missing answer, a wrong kind, a + // missing member, a value outside what was asked, or a number outside + // its range is refused, and the whole call fails (all-or-nothing, per + // [Provider.Ask]). Each of those would otherwise decode into a plausible + // value the provider never meant — see checkAnswer. + // + // The result is built from the QUESTIONS, not from the response, so an + // unsolicited extra answer is dropped rather than returned: the contract + // is one entry per question key. + answers := make(map[string]Answer, len(questions)) + for key, q := range questions { + wa, ok := resp.Answers[key] + if !ok { + return nil, Usage{}, fmt.Errorf("decision: provider returned no answer for question %q", key) + } + if err := checkAnswer(q, wa); err != nil { + return nil, Usage{}, fmt.Errorf("decision: question %q: %w", key, err) + } + answers[key] = fromWire(wa) + } + + return answers, Usage{ + InputTokens: resp.Usage.InputTokens, + OutputTokens: resp.Usage.OutputTokens, + StateTruncated: truncated, + }, nil +} + +// toWire converts a question to its provider representation. Criteria shape +// is per kind: option->description for choice, an ordered array for score, +// and a true/false pair (omitted when both are empty) for noul. +func toWire(q Question) wireQuestion { + w := wireQuestion{Type: string(q.Kind), Instructions: q.Instructions} + switch q.Kind { + case KindChoice: + w.Criteria = q.Options + case KindScore: + w.Criteria = q.Levels + case KindNoul: + if q.TrueDesc != "" || q.FalseDesc != "" { + w.Criteria = map[string]string{"true": q.TrueDesc, "false": q.FalseDesc} + } + } + return w +} + +// fromWire converts a provider answer that checkAnswer has accepted, keeping +// only the members the answered kind populates. Copying a stray Score onto a +// choice answer would hand a consumer a number the provider never meant. +func fromWire(w wireAnswer) Answer { + a := Answer{Kind: Kind(w.Type)} + switch a.Kind { + case KindChoice: + a.Choice = *w.Choice + a.Probabilities = w.Probabilities + a.Confidence = w.Confidence + case KindScore: + a.Score = *w.Score + a.Legend = w.Legend + a.Probabilities = w.Probabilities + a.Confidence = w.Confidence + case KindNoul: + a.Noul = *w.Noul + } + return a +} + +// checkAnswer refuses an answer that contradicts the question it answers. +// +// The population is every (kind, member) cell of [Answer], enumerated in +// the table on TASK-3116's trail (review round 2). Per kind: +// +// - every kind: Type must equal the question's kind. +// - choice: choice PRESENT and one of the options sent; confidence PRESENT +// and in [0,1]; probabilities NON-EMPTY, keyed only by options sent, +// each in [0,1]. +// - score: score PRESENT and in [0, levels-1] (it is the probability- +// weighted mean over level indices, so it cannot leave that range); +// confidence PRESENT and in [0,1]; probabilities NON-EMPTY; probability +// and legend keys each the canonical decimal index of a level sent +// ("0".."n-1", so "01" and "9" are refused); probabilities in [0,1]. +// - noul: noul PRESENT and in [0,1]. Members noul does not use are +// ignored rather than refused — fromWire drops them, so they cannot +// reach a caller. +// +// What it does NOT check: that probabilities sum to 1 (the provider rounds, +// and nothing downstream depends on the sum), or that the legend's NAMES +// match the levels sent (they are echoed text; a mismatch misnames, it +// cannot mis-index). +func checkAnswer(q Question, w wireAnswer) error { + if Kind(w.Type) != q.Kind { + return fmt.Errorf("asked a %s but the provider answered with a %s", q.Kind, w.Type) + } + switch q.Kind { + case KindChoice: + if w.Choice == nil { + return errors.New("choice answer carries no choice") + } + if _, ok := q.Options[*w.Choice]; !ok { + return fmt.Errorf("provider chose %q, which is not one of the options sent", *w.Choice) + } + if err := checkConfidence(w.Confidence); err != nil { + return err + } + return checkProbabilities(w.Probabilities, func(k string) bool { + _, ok := q.Options[k] + return ok + }) + case KindScore: + if w.Score == nil { + return errors.New("score answer carries no score") + } + if hi := float64(len(q.Levels) - 1); *w.Score < 0 || *w.Score > hi { + return fmt.Errorf("score %v is outside [0, %v] for %d levels", *w.Score, hi, len(q.Levels)) + } + if err := checkConfidence(w.Confidence); err != nil { + return err + } + isLevel := func(k string) bool { + n, err := strconv.Atoi(k) + return err == nil && n >= 0 && n < len(q.Levels) && strconv.Itoa(n) == k + } + for k := range w.Legend { + if !isLevel(k) { + return fmt.Errorf("legend key %q is not the index of a level sent", k) + } + } + return checkProbabilities(w.Probabilities, isLevel) + case KindNoul: + if w.Noul == nil { + return errors.New("noul answer carries no noul value") + } + if !inUnit(*w.Noul) { + return fmt.Errorf("noul %v is outside [0, 1]", *w.Noul) + } + } + return nil +} + +func checkConfidence(c *float64) error { + if c == nil { + return errors.New("answer carries no confidence") + } + if !inUnit(*c) { + return fmt.Errorf("confidence %v is outside [0, 1]", *c) + } + return nil +} + +func checkProbabilities(p map[string]float64, validKey func(string) bool) error { + if len(p) == 0 { + return errors.New("answer carries no probabilities") + } + for k, v := range p { + if !validKey(k) { + return fmt.Errorf("probability key %q is not an outcome that was asked", k) + } + if !inUnit(v) { + return fmt.Errorf("probability %v for %q is outside [0, 1]", v, k) + } + } + return nil +} + +func inUnit(f float64) bool { return f >= 0 && f <= 1 } + +// fitState shortens a STRING state, if needed, so that the request's +// estimated size fits the measured token budget. It reports whether it +// truncated. The estimate is bytes over [charsPerToken], which is a +// conservative heuristic rather than a bound — see that constant — so a +// fitted request can still be refused by the provider, and that refusal is +// surfaced as [ErrMaxTokensExceeded]. +// +// Only a STRING state is truncated. Cutting a marshalled struct or map at a +// byte boundary would produce invalid JSON, and silently dropping fields +// would change what the questions are being asked ABOUT; a caller with a +// structured state that does not fit gets the provider's own refusal, which +// names the problem correctly. +func (p *typesafeProvider) fitState(state any, questions map[string]wireQuestion) (any, bool, error) { + // Reserve exactly what everything but the state costs, by marshalling + // the request with an empty string state: model name, questions and + // punctuation, whatever their size. A fixed allowance here let a long + // model name push a cut-to-the-limit request over the budget. + envelope, err := json.Marshal(wireRequest{Model: p.model, State: "", Questions: questions}) + if err != nil { + return nil, false, fmt.Errorf("decision: marshal questions: %w", err) + } + budgetChars := int(float64(maxRequestTokens) * charsPerToken) + // remaining is what the state VALUE may occupy, its quotes included — + // the envelope's own "" is subtracted back out because the state's + // marshalled length counts it again. + remaining := budgetChars - (len(envelope) - 2) + if remaining < 0 { + return nil, false, fmt.Errorf("decision: questions alone exceed the %d-token budget", maxRequestTokens) + } + + s, ok := state.(string) + if !ok || marshalledLen(s) <= remaining { + return state, false, nil + } + // A string state that does not fit, with fewer than two bytes of room: + // not even "" fits, and returning it would still send a request one or + // two bytes over (review round 3). This sits AFTER the string check on + // purpose — a structured state is never cut, and one as small as `0` + // fits in a single byte (review round 4). + if remaining < 2 { + return nil, false, fmt.Errorf("decision: no room for the state beside the questions: %d byte(s) left, "+ + "and even an empty string needs 2", remaining) + } + + // The budget is measured against the MARSHALLED length, not the raw + // string length, because encoding/json escapes <, > and & to six-byte + // \uXXXX sequences: a state of 80,000 '<' characters is 80,000 raw bytes + // and 480,000 on the wire (measured ratio exactly 6.00). Sizing the cut + // on raw length let such a state sail past the budget check and come back + // as max_tokens_exceeded, which is the very refusal truncation exists to + // avoid. + // + // Expansion is not uniform, so one division cannot place the cut. Start + // from the raw budget — marshalled length is never less than raw length, + // so that is a valid upper bound — then shrink by the expansion ratio + // actually observed at the current cut until it fits. `cut` strictly + // decreases every iteration, so this terminates. + // + // The first cut leaves room for the two quote bytes the marshalled + // string adds, so a state with no escapes fits on the first try instead + // of overshooting by two and being scaled back by the 5% margin below. + cut := runeBoundaryAtOrBefore(s, min(remaining-2, len(s))) + for cut > 0 { + got := marshalledLen(s[:cut]) + if got <= remaining { + return s[:cut], true, nil + } + // Scale by the observed ratio, with a margin so a pathological + // distribution still converges, and always make progress. + next := int(float64(cut) * (float64(remaining) / float64(got)) * 0.95) + if next >= cut { + next = cut - 1 + } + cut = runeBoundaryAtOrBefore(s, next) + } + // Nothing of the state fits beside the questions. + return "", true, nil +} + +// marshalledLen is the number of bytes this string occupies as a JSON value +// — the bytes actually sent, which are what the token estimate is taken over. +func marshalledLen(s string) int { + b, err := json.Marshal(s) + if err != nil { + // json.Marshal cannot fail for a string; fall back to a + // deliberately pessimistic bound rather than reporting a small one. + return len(s) * 6 + } + return len(b) +} + +// runeBoundaryAtOrBefore returns the largest index <= i that is not a UTF-8 +// continuation byte, so a cut there never SPLITS a rune: valid UTF-8 input +// yields a valid prefix. It does not repair input that was already invalid — +// a byte like 0xff counts as a boundary — and nothing here does: json.Marshal +// replaces invalid bytes with U+FFFD whether or not the state was cut, and +// marshalledLen measures the replaced form, so the budget still holds. +func runeBoundaryAtOrBefore(s string, i int) int { + if i < 0 { + return 0 + } + // len(s) is a valid cut point (the whole string) and indexing s[len(s)] + // would panic, so return early rather than clamping into the loop. + if i >= len(s) { + return len(s) + } + for i > 0 && !isRuneStart(s[i]) { + i-- + } + return i +} + +// isRuneStart reports whether b can begin a UTF-8 sequence — i.e. it is not a +// continuation byte. +func isRuneStart(b byte) bool { return b&0xC0 != 0x80 } + +// post sends the request, retrying transient failures with exponential +// backoff, and returns the decoded response. +func (p *typesafeProvider) post(ctx context.Context, body []byte) (*wireResponse, error) { + backoff := initialBackoff + var lastErr error + + for attempt := 0; attempt < maxAttempts; attempt++ { + if attempt > 0 { + wait := backoff + if ra := retryAfterOf(lastErr); ra > 0 { + wait = ra + } + // The wait itself is cancellable; a cancelled context must not + // have to outlast the backoff before anyone notices. + if err := p.sleep(ctx, wait); err != nil { + return nil, err + } + if backoff *= 2; backoff > maxBackoff { + backoff = maxBackoff + } + } + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, p.endpoint, bytes.NewReader(body)) + if err != nil { + return nil, fmt.Errorf("decision: create request: %w", err) + } + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", "Bearer "+p.apiKey) + + resp, err := p.client.Do(req) + if err != nil { + // A transport error may be transient; keep retrying, but never + // past a cancelled context. + if ctx.Err() != nil { + return nil, ctx.Err() + } + lastErr = fmt.Errorf("decision: call provider: %w", err) + continue + } + + raw, readErr := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) + retryAfter := parseRetryAfter(resp.Header.Get("Retry-After")) + status := resp.StatusCode + resp.Body.Close() + + switch { + case status == http.StatusTooManyRequests || status == 529: + // Retryable by the provider's own contract. + lastErr = &transientError{APIError: apiErrorFrom(status, raw, readErr), retryAfter: retryAfter} + continue + case status >= 500: + lastErr = &transientError{APIError: apiErrorFrom(status, raw, readErr)} + continue + case status >= 400: + // Not retryable: the request itself is the problem. The body is + // surfaced verbatim, and max_tokens_exceeded is matchable via + // errors.Is without parsing prose. A body read that failed + // partway is carried on the error rather than dropped, so a + // caller can tell a missing ErrorType from an unread one. + apiErr := apiErrorFrom(status, raw, readErr) + return nil, &apiErr + } + + if readErr != nil { + return nil, fmt.Errorf("decision: read provider response: %w", readErr) + } + var out wireResponse + if err := json.Unmarshal(raw, &out); err != nil { + return nil, fmt.Errorf("decision: decode provider response: %w", err) + } + if len(out.Answers) == 0 { + return nil, errors.New("decision: provider returned no answers") + } + return &out, nil + } + + if lastErr == nil { + lastErr = errors.New("decision: provider retries exhausted") + } + return nil, fmt.Errorf("decision: provider failed after %d attempts: %w", maxAttempts, lastErr) +} + +// transientError wraps a retryable provider response so the retry loop can +// carry a Retry-After hint without widening APIError. +type transientError struct { + APIError + retryAfter time.Duration +} + +// Unwrap exposes the embedded APIError to errors.As. +// +// Without it, exhausted retries produce an error chain of +// fmt.Errorf -> *transientError and stop: errors.As(err, **APIError) returns +// FALSE, because *transientError is a different type and value-embedding does +// not make it one. A caller that retried four times into a 429 wall would then +// have no way to read the status or the error type off the failure — the +// information is in the chain and unreachable. +func (t *transientError) Unwrap() error { return &t.APIError } + +// apiErrorFrom builds an APIError from a response, keeping a partial-read +// failure rather than discarding it. +func apiErrorFrom(status int, raw []byte, readErr error) APIError { + return APIError{ + StatusCode: status, + ErrorType: errorTypeOf(raw), + Body: boundedBody(raw), + BodyReadErr: readErr, + } +} + +func retryAfterOf(err error) time.Duration { + var t *transientError + if errors.As(err, &t) { + return t.retryAfter + } + return 0 +} + +// parseRetryAfter reads a Retry-After header in delta-seconds form, bounded +// so a hostile or mistaken value cannot park a request indefinitely. +func parseRetryAfter(v string) time.Duration { + v = strings.TrimSpace(v) + if v == "" { + return 0 + } + secs, err := strconv.Atoi(v) + if err != nil || secs <= 0 { + return 0 + } + d := time.Duration(secs) * time.Second + if d > maxRetryAfter { + return maxRetryAfter + } + return d +} + +// errorTypeOf extracts detail.error_type from an error body, returning "" if +// the body is not that shape. It tolerates detail being a string or absent, +// since only the object form is measured and the rest is undocumented. +func errorTypeOf(raw []byte) string { + var we wireError + if err := json.Unmarshal(raw, &we); err != nil { + return "" + } + m, ok := we.Detail.(map[string]any) + if !ok { + return "" + } + s, _ := m["error_type"].(string) + return s +} + +func boundedBody(raw []byte) string { + s := strings.TrimSpace(string(raw)) + if len(s) > maxErrBodyLength { + return s[:maxErrBodyLength] + "...(truncated)" + } + return s +} + +// levelIndex parses a score probability/legend key ("0", "1", ...) into its +// integer index. A key that is not an integer sorts last rather than +// panicking, so an unexpected vocabulary degrades instead of crashing. +func levelIndex(k string) int { + n, err := strconv.Atoi(k) + if err != nil { + return 1 << 30 + } + return n +}