diff --git a/cmd/zero-perf-bench/turn.go b/cmd/zero-perf-bench/turn.go index 7009bebe0..344de58d2 100644 --- a/cmd/zero-perf-bench/turn.go +++ b/cmd/zero-perf-bench/turn.go @@ -9,6 +9,7 @@ import ( "path/filepath" "strings" + "github.com/Gitlawb/zero/internal/execprofile" "github.com/Gitlawb/zero/internal/perfbench" ) @@ -20,6 +21,7 @@ type turnOptions struct { SuitePath string Model string Mode string + ExecProfile string SelfCorrect bool Binary string Iterations int @@ -64,6 +66,7 @@ func runTurnCommand(args []string, getenv func(string) string, stdout io.Writer, result, err := perfbench.RunTurnBench(context.Background(), set, perfbench.TurnBenchConfig{ Model: options.Model, Mode: options.Mode, + ExecProfile: options.ExecProfile, SelfCorrect: options.SelfCorrect, Version: options.Version, Commit: options.Commit, @@ -137,6 +140,13 @@ func parseTurnArgs(args []string, getenv func(string) string) (turnOptions, erro } options.Mode = value index = next + case "--exec-profile": + value, next, err := readOptionValue(args, inlineValue, index, flag) + if err != nil { + return options, err + } + options.ExecProfile = value + index = next case "--binary": value, next, err := readOptionValue(args, inlineValue, index, flag) if err != nil { @@ -209,6 +219,20 @@ func parseTurnArgs(args []string, getenv func(string) string) (turnOptions, erro if strings.TrimSpace(options.Model) == "" && !options.DryRun { return options, fmt.Errorf("--model is required (or pass --dry-run)") } + // Validate the profile here, before anything spawns: an unknown name would + // otherwise make every child exit with a usage error in milliseconds, and + // those near-zero walls would be recorded as valid latency samples in an + // exit-0 report — exactly the misread-as-improvement trap the harness + // closed for spawn failures. Normalizing to the catalog name also keeps + // the stamped execProfile canonical (Lookup is case-insensitive), so two + // captures of the same posture always compare equal. + if raw := strings.TrimSpace(options.ExecProfile); raw != "" { + profile, ok := execprofile.Lookup(raw) + if !ok { + return options, fmt.Errorf("unknown execution profile %q for --exec-profile. Valid profiles: %s", raw, strings.Join(execprofile.Names(), ", ")) + } + options.ExecProfile = profile.Name + } return options, nil } @@ -239,6 +263,9 @@ func turnHelpText() string { " --suite Task set JSON file (required)", " --model Model to run (required unless --dry-run)", " --mode Exec mode preset to apply", + " --exec-profile ", + " Execution profile for every task (fast|balanced|thorough);", + " forwarded to zero exec and stamped into the result", " --self-correct Enable the post-edit verify-and-correct loop", " --binary Path to the `zero` binary (default: zero on PATH / repo root)", " --iterations Times to run each task (default: 1)", diff --git a/cmd/zero-perf-bench/turn_test.go b/cmd/zero-perf-bench/turn_test.go new file mode 100644 index 000000000..e7035e12b --- /dev/null +++ b/cmd/zero-perf-bench/turn_test.go @@ -0,0 +1,65 @@ +package main + +import ( + "strings" + "testing" +) + +func TestParseTurnArgsExecProfile(t *testing.T) { + getenv := func(string) string { return "" } + options, err := parseTurnArgs([]string{ + "--suite", "suite.json", + "--model", "m", + "--exec-profile", "fast", + }, getenv) + if err != nil { + t.Fatalf("parseTurnArgs: %v", err) + } + if options.ExecProfile != "fast" { + t.Fatalf("ExecProfile = %q, want fast", options.ExecProfile) + } + + // Inline form. + options, err = parseTurnArgs([]string{ + "--suite=suite.json", + "--model=m", + "--exec-profile=thorough", + }, getenv) + if err != nil { + t.Fatalf("parseTurnArgs inline: %v", err) + } + if options.ExecProfile != "thorough" { + t.Fatalf("ExecProfile = %q, want thorough", options.ExecProfile) + } +} + +// The bench validates the profile BEFORE spawning anything: an unknown name +// would otherwise make every child exit with a usage error in milliseconds +// and record those near-zero walls as valid latency samples in an exit-0 +// report. Lookup normalization also keeps the stamped name canonical so two +// captures of the same posture compare equal. +func TestParseTurnArgsExecProfileValidatesAndNormalizes(t *testing.T) { + getenv := func(string) string { return "" } + + options, err := parseTurnArgs([]string{ + "--suite", "suite.json", "--model", "m", "--exec-profile", "FAST", + }, getenv) + if err != nil { + t.Fatalf("parseTurnArgs: %v", err) + } + if options.ExecProfile != "fast" { + t.Fatalf("ExecProfile = %q, want the canonical fast", options.ExecProfile) + } + + _, err = parseTurnArgs([]string{ + "--suite", "suite.json", "--model", "m", "--exec-profile", "blanced", + }, getenv) + if err == nil { + t.Fatal("an unknown profile must fail parse, before anything spawns") + } + for _, name := range []string{"balanced", "fast", "thorough"} { + if !strings.Contains(err.Error(), name) { + t.Fatalf("error must list %q, got %q", name, err.Error()) + } + } +} diff --git a/internal/agent/loop.go b/internal/agent/loop.go index 8e05b84cd..98554c034 100644 --- a/internal/agent/loop.go +++ b/internal/agent/loop.go @@ -202,6 +202,8 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options) } if target.ReasoningEffort != "" { options.ReasoningEffort = target.ReasoningEffort + } else if target.RestoreDefaultEffort { + options.ReasoningEffort = "" } if target.RestoreCompletionGate { options.RequireCompletionSignal = true diff --git a/internal/agent/profile_controller_test.go b/internal/agent/profile_controller_test.go index 3783ece05..5ce966d8d 100644 --- a/internal/agent/profile_controller_test.go +++ b/internal/agent/profile_controller_test.go @@ -153,6 +153,51 @@ func TestPostureEscalationOnUncertainCompletion(t *testing.T) { } } +// A profile that filled the run's effort displaced the provider default (""); +// RestoreDefaultEffort lets escalation restore that default, which a plain "" +// ReasoningEffort target cannot express (it means "leave untouched"). +func TestPostureEscalationRestoresDefaultEffort(t *testing.T) { + provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{ + { + {Type: zeroruntime.StreamEventText, Content: "Let me read the file:"}, + {Type: zeroruntime.StreamEventDone}, + }, + { + {Type: zeroruntime.StreamEventText, Content: "Done. All set."}, + {Type: zeroruntime.StreamEventDone}, + }, + }} + + result, err := Run(context.Background(), "go", provider, Options{ + Registry: tools.NewRegistry(), + MaxTurns: 10, + ReasoningEffort: "low", + RequireCompletionSignal: true, + Profile: &ProfilePolicy{ + Name: "fast", + Escalate: &PostureEscalation{ + RestoreDefaultEffort: true, + OnCompletionUncertain: 1, + }, + }, + }) + if err != nil { + t.Fatal(err) + } + if result.FinalAnswer != "Done. All set." { + t.Fatalf("expected the completed answer, got %q", result.FinalAnswer) + } + if len(provider.requests) != 2 { + t.Fatalf("expected 2 requests, got %d", len(provider.requests)) + } + if provider.requests[0].ReasoningEffort != "low" { + t.Fatalf("pre-escalation request effort = %q, want the profile's low", provider.requests[0].ReasoningEffort) + } + if provider.requests[1].ReasoningEffort != "" { + t.Fatalf("post-escalation request effort = %q, want the restored provider default (empty)", provider.requests[1].ReasoningEffort) + } +} + // failingProfileTool always errors with the same signature so the repeated- // failure guard builds a streak. type failingProfileTool struct{} diff --git a/internal/agent/types.go b/internal/agent/types.go index 28f1fdb5f..7055b3ecf 100644 --- a/internal/agent/types.go +++ b/internal/agent/types.go @@ -126,6 +126,12 @@ type PostureEscalation struct { MaxTurns int // ReasoningEffort replaces the run's effort when non-empty. ReasoningEffort string + // RestoreDefaultEffort clears the run's effort override (back to the + // provider/model default) when true. It exists because the displaced value + // of a profile-filled effort is "" — which as a ReasoningEffort target + // means "leave untouched" — so restoring the default needs its own signal. + // Ignored when ReasoningEffort is non-empty. + RestoreDefaultEffort bool // RestoreCompletionGate re-enables RequireCompletionSignal (headless // completion semantics) when true. RestoreCompletionGate bool diff --git a/internal/cli/app.go b/internal/cli/app.go index 07769724a..24002236a 100644 --- a/internal/cli/app.go +++ b/internal/cli/app.go @@ -1341,6 +1341,9 @@ Flags: --spec-reasoning-effort Override draft reasoning effort when --use-spec is set --max-turns Override the maximum agent loop turns + --exec-profile Apply an execution profile (balanced, fast, thorough): loop + posture only (turn budget, effort, self-correction, escalation); + composes with --mode (which picks the model) and explicit flags win --auto Set exec autonomy; high enables unsafe tools --enabled-tools Only expose these comma or space separated tools --disabled-tools Hide these comma or space separated tools diff --git a/internal/cli/exec.go b/internal/cli/exec.go index 18b60d21a..239c6bd51 100644 --- a/internal/cli/exec.go +++ b/internal/cli/exec.go @@ -13,6 +13,7 @@ import ( "github.com/Gitlawb/zero/internal/agent" "github.com/Gitlawb/zero/internal/config" "github.com/Gitlawb/zero/internal/errhint" + "github.com/Gitlawb/zero/internal/execprofile" "github.com/Gitlawb/zero/internal/imageinput" "github.com/Gitlawb/zero/internal/lsp" "github.com/Gitlawb/zero/internal/modelregistry" @@ -69,7 +70,14 @@ type execOptions struct { // intentionally inert: nothing consumes it. Model selection is driven by // --model / --mode instead. See writeExecHelp ("Accept legacy model profile // selection") and TestRunExecAcceptsLegacyModelProfileFlags. - modelProfile string + modelProfile string + // execProfile selects a named execution profile (balanced, fast, thorough): + // a loop-posture bundle (turn budget, reasoning effort, self-correction, + // escalation triggers) applied AFTER --mode with the same + // fill-only-if-unset rule, so precedence is explicit flag > mode > profile. + // Distinct from the mode preset also named "fast" (which picks a model) and + // from the legacy inert --profile above. See internal/execprofile. + execProfile string reasoningEffort string useSpec bool specModel string @@ -167,6 +175,14 @@ func runExec(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) in if err := applyExecMode(&options); err != nil { return writeExecFormatUsageError(stdout, stderr, options.outputFormat, err.Error()) } + // A profile tunes loop posture the way a mode tunes model selection, and + // runs after it so the mode's fills count as "set" and win. MaxTurns is + // deferred to config resolution below, where the displaced resolved budget + // is known and becomes the escalation restore target. + execProfile, execProfileFilledEffort, err := applyExecProfile(&options) + if err != nil { + return writeExecFormatUsageError(stdout, stderr, options.outputFormat, err.Error()) + } workspaceRoot, err := resolveWorkspaceRoot(options.cwd, deps) if err != nil { @@ -272,6 +288,8 @@ func runExec(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) in } return writeExecProviderError(stdout, stderr, options.outputFormat, "provider_error", err.Error()) } + var displacedMaxTurns int + resolved.MaxTurns, displacedMaxTurns = applyProfileTurnBudget(execProfile, options.maxTurns, resolved.MaxTurns) registerLocalControlTools(registry, workspaceRoot, resolved.LocalControl) if err := validateExecToolFilters(options, registry); err != nil { return writeExecFormatUsageError(stdout, stderr, options.outputFormat, err.Error()) @@ -486,6 +504,13 @@ func runExec(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) in reasoningEffort: forwardEffort, specPermissionMode: permissionMode, notifier: notifier, + // The profile displaced resolved.MaxTurns above, so the spec-draft + // run arms the same escalation policy as the main run (in practice + // only the failure-streak and risky-mutation triggers can fire + // here: the draft runs without the completion gate or a + // self-corrector). + profilePolicy: execProfile.Policy(displacedMaxTurns, + specProfileEffortFilled(execProfileFilledEffort, options.specReasoningEffort)), }) } @@ -528,7 +553,7 @@ func runExec(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) in var traceRecorder *trace.Recorder var traceSnapshot *trace.TurnTrace if tracePath != "" { - traceRecorder = trace.NewRecorder(preparedSession.Session.SessionID, runID, "") + traceRecorder = trace.NewRecorder(preparedSession.Session.SessionID, runID, execProfile.Name) defer func() { if err := writeTraceSnapshot(traceSnapshot, tracePath, stderr); err != nil { fmt.Fprintf(stderr, "[zero] failed to write trace: %s\n", err) @@ -627,6 +652,7 @@ func runExec(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) in Autonomy: options.autonomy, SelfCorrect: selfCorrector, FileDiagnostics: fileDiagnostics, + Profile: execProfile.Policy(displacedMaxTurns, execProfileFilledEffort), // Headless exec: don't accept a no-tool-call turn as "done" while work // clearly remains (pending plan items / a mid-step continuation cue) — // nudge to continue, and finalize as INCOMPLETE rather than false success @@ -1133,6 +1159,67 @@ func applyExecMode(options *execOptions) error { return nil } +// applyExecProfile expands an --exec-profile selection onto the exec options. +// Like applyExecMode it only fills fields the caller left unset, and it runs +// AFTER the mode so a mode-filled field counts as set: precedence is explicit +// flag > --mode > --exec-profile. Only the options-level knobs are applied here +// (reasoning effort still flows through the model-supported gating downstream; +// self-correct is presence-only so a profile can only turn it on). MaxTurns is +// intentionally NOT applied here — the profile displaces the resolved budget +// after config resolution, where the displaced value is known and becomes the +// escalation restore target. An unknown profile is a usage error listing the +// valid names. Not to be confused with the legacy inert --profile flag. +// The second return reports whether the profile actually filled the reasoning +// effort (false when the user set one explicitly), so the escalation policy +// knows the displaced effort was the provider default and can restore it. +func applyExecProfile(options *execOptions) (execprofile.Profile, bool, error) { + name := strings.TrimSpace(options.execProfile) + if name == "" { + return execprofile.Profile{}, false, nil + } + profile, ok := execprofile.Lookup(name) + if !ok { + return execprofile.Profile{}, false, execUsageError{fmt.Sprintf("unknown execution profile %q. Valid profiles: %s.", options.execProfile, strings.Join(execprofile.Names(), ", "))} + } + effortFilled := false + if options.reasoningEffort == "" && profile.ReasoningEffort != "" { + options.reasoningEffort = profile.ReasoningEffort + effortFilled = true + } + // Spec-safe projection: the spec-draft path wires no self-corrector (an + // explicit --self-correct --use-spec is rejected at parse time, before + // profiles apply), so a profile's self-correct knob is meaningless there. + // Project it away rather than erroring: the user asked for a thorough + // DRAFT, and the profile's other knobs (budget, effort) apply to it fine. + if profile.SelfCorrect && !options.useSpec { + options.selfCorrect = true + } + return profile, effortFilled, nil +} + +// specProfileEffortFilled reports whether the profile's effort fill actually +// governs the spec-draft run. An explicit --spec-reasoning-effort replaces the +// filled effort for the draft, so the escalation's effort restore must not arm +// there: escalation must never clear an effort the user pinned by hand. +func specProfileEffortFilled(effortFilled bool, specReasoningEffort string) bool { + return effortFilled && strings.TrimSpace(specReasoningEffort) == "" +} + +// applyProfileTurnBudget decides the run's turn budget once config is resolved. +// The profile displaces the RESOLVED value, not the flag: a pinned budget +// always wins and the profile backs off entirely, while an env/config budget +// is the "balanced posture" a mid-run escalation can restore. pinnedMaxTurns +// is any caller-pinned budget — an explicit --max-turns flag OR a mode +// preset's fill (both land in options.maxTurns, and a mode outranks a profile +// just like the flag does). displaced is 0 when nothing was displaced, so an +// escalation leaves the ceiling untouched. +func applyProfileTurnBudget(profile execprofile.Profile, pinnedMaxTurns int, resolvedMaxTurns int) (effective int, displaced int) { + if profile.MaxTurns > 0 && pinnedMaxTurns == 0 { + return profile.MaxTurns, resolvedMaxTurns + } + return resolvedMaxTurns, 0 +} + // resolveSelectedModel routes a user-supplied --model value through the model // registry so that fuzzy aliases (e.g. "sonnet 4.5") resolve to canonical ids // and deprecated models auto-redirect to their fallback. It returns the model id diff --git a/internal/cli/exec_parse.go b/internal/cli/exec_parse.go index 907cababc..8eb55223d 100644 --- a/internal/cli/exec_parse.go +++ b/internal/cli/exec_parse.go @@ -147,6 +147,19 @@ func parseExecArgs(args []string) (execOptions, bool, error) { index = next case strings.HasPrefix(arg, "--profile="): options.modelProfile = strings.TrimSpace(strings.TrimPrefix(arg, "--profile=")) + case arg == "--exec-profile": + value, next, err := nextFlagValue(args, index, arg) + if err != nil { + return options, false, err + } + options.execProfile = strings.TrimSpace(value) + index = next + case strings.HasPrefix(arg, "--exec-profile="): + value, err := requiredInlineFlagValue(arg, "--exec-profile") + if err != nil { + return options, false, err + } + options.execProfile = value case arg == "-r" || arg == "--reasoning-effort": value, next, err := nextFlagValue(args, index, arg) if err != nil { diff --git a/internal/cli/exec_profile_test.go b/internal/cli/exec_profile_test.go new file mode 100644 index 000000000..80d5308fa --- /dev/null +++ b/internal/cli/exec_profile_test.go @@ -0,0 +1,224 @@ +package cli + +import ( + "os" + "path/filepath" + "reflect" + "strings" + "testing" + + "github.com/Gitlawb/zero/internal/execprofile" +) + +func TestApplyExecProfileFillsOnlyUnset(t *testing.T) { + // Unset effort: the profile fills it and reports the fill, so the + // escalation policy knows the displaced effort was the provider default. + options := execOptions{execProfile: "fast"} + profile, effortFilled, err := applyExecProfile(&options) + if err != nil { + t.Fatalf("applyExecProfile: %v", err) + } + if profile.Name != "fast" { + t.Fatalf("profile.Name = %q, want fast", profile.Name) + } + if options.reasoningEffort != "low" { + t.Fatalf("reasoningEffort = %q, want the profile's low", options.reasoningEffort) + } + if !effortFilled { + t.Fatal("the profile filled the effort, so effortFilled must report it") + } + + // Set effort (explicit flag or a mode's fill): the profile backs off. + options = execOptions{execProfile: "fast", reasoningEffort: "high"} + _, effortFilled, err = applyExecProfile(&options) + if err != nil { + t.Fatalf("applyExecProfile: %v", err) + } + if options.reasoningEffort != "high" { + t.Fatalf("reasoningEffort = %q, an explicit value must win over the profile", options.reasoningEffort) + } + if effortFilled { + t.Fatal("the profile backed off, so effortFilled must be false") + } +} + +func TestApplyExecProfileThoroughArmsSelfCorrect(t *testing.T) { + options := execOptions{execProfile: "thorough"} + if _, _, err := applyExecProfile(&options); err != nil { + t.Fatalf("applyExecProfile: %v", err) + } + if !options.selfCorrect { + t.Fatal("thorough must arm the post-edit self-correct loop") + } + if options.reasoningEffort != "high" { + t.Fatalf("reasoningEffort = %q, want high", options.reasoningEffort) + } +} + +// The spec-draft path wires no self-corrector (explicit --self-correct +// --use-spec is rejected at parse time, before profiles apply), so a profile's +// self-correct knob is projected away under --use-spec instead of silently +// arming a knob the draft runner ignores. The other knobs still apply. +func TestApplyExecProfileSpecProjectsAwaySelfCorrect(t *testing.T) { + options := execOptions{execProfile: "thorough", useSpec: true} + if _, _, err := applyExecProfile(&options); err != nil { + t.Fatalf("applyExecProfile: %v", err) + } + if options.selfCorrect { + t.Fatal("thorough under --use-spec must not arm self-correct (the draft runner has none)") + } + if options.reasoningEffort != "high" { + t.Fatalf("reasoningEffort = %q, the profile's other knobs must still apply", options.reasoningEffort) + } +} + +func TestApplyExecProfileUnknownIsUsageError(t *testing.T) { + options := execOptions{execProfile: "turbo"} + _, _, err := applyExecProfile(&options) + if err == nil { + t.Fatal("expected a usage error for an unknown profile") + } + if _, ok := err.(execUsageError); !ok { + t.Fatalf("expected execUsageError, got %T: %v", err, err) + } + for _, name := range execprofile.Names() { + if !strings.Contains(err.Error(), name) { + t.Fatalf("usage error must list %q, got %q", name, err.Error()) + } + } +} + +// Precedence: explicit flag > --mode > --exec-profile. The deep mode fills +// effort=high and max-turns=160; the fast profile must not override either, +// because a mode-filled field counts as set by the time the profile runs. +func TestExecProfilePrecedenceFlagOverModeOverProfile(t *testing.T) { + options := execOptions{mode: "deep", execProfile: "fast"} + if err := applyExecMode(&options); err != nil { + t.Fatalf("applyExecMode: %v", err) + } + profile, _, err := applyExecProfile(&options) + if err != nil { + t.Fatalf("applyExecProfile: %v", err) + } + if options.reasoningEffort != "high" { + t.Fatalf("reasoningEffort = %q, the mode's high must win over the profile's low", options.reasoningEffort) + } + // The mode filled options.maxTurns, so the turn budget must not be displaced. + effective, displaced := applyProfileTurnBudget(profile, options.maxTurns, options.maxTurns) + if effective != 160 || displaced != 0 { + t.Fatalf("turn budget = (%d, displaced %d), the mode's 160 must win with nothing displaced", effective, displaced) + } +} + +func TestApplyProfileTurnBudget(t *testing.T) { + fast, _ := execprofile.Lookup("fast") + balanced, _ := execprofile.Lookup("balanced") + + // No explicit flag: the profile displaces the resolved budget. + if effective, displaced := applyProfileTurnBudget(fast, 0, 80); effective != 30 || displaced != 80 { + t.Fatalf("fast over resolved 80 = (%d, %d), want (30, 80)", effective, displaced) + } + // Explicit --max-turns pins the budget; the profile backs off entirely. + if effective, displaced := applyProfileTurnBudget(fast, 50, 50); effective != 50 || displaced != 0 { + t.Fatalf("fast with explicit 50 = (%d, %d), want (50, 0)", effective, displaced) + } + // Balanced never displaces anything. + if effective, displaced := applyProfileTurnBudget(balanced, 0, 80); effective != 80 || displaced != 0 { + t.Fatalf("balanced over resolved 80 = (%d, %d), want (80, 0)", effective, displaced) + } +} + +// The no-regression invariant at the options level: selecting balanced leaves +// the options byte-identical to not selecting a profile at all, and produces +// no loop policy. +func TestExecProfileBalancedLeavesOptionsUntouched(t *testing.T) { + options := execOptions{execProfile: "balanced"} + reference := options + profile, _, err := applyExecProfile(&options) + if err != nil { + t.Fatalf("applyExecProfile: %v", err) + } + if !reflect.DeepEqual(options, reference) { + t.Fatalf("balanced changed the options: %+v vs %+v", options, reference) + } + if policy := profile.Policy(80, false); policy != nil { + t.Fatalf("balanced must produce a nil policy, got %+v", policy) + } +} + +// --exec-profile= with an inline empty value must error like its two-token +// form and the sibling --mode=, not silently run the default posture (the +// classic shell hazard: an unset variable expanding into the inline form). +func TestParseExecProfileInlineEmptyIsError(t *testing.T) { + _, _, err := parseExecArgs([]string{"--exec-profile=", "hello"}) + if err == nil || !strings.Contains(err.Error(), "--exec-profile requires a value") { + t.Fatalf("expected a required-value error, got %v", err) + } +} + +// Full echo-provider run: the selected profile must be stamped into the +// per-turn trace so benchmark attribution can group runs by posture. +func TestRunExecTraceRecordsSelectedProfile(t *testing.T) { + tracePath := filepath.Join(t.TempDir(), "trace.ndjson") + exitCode, _, stderr := runExecWithEcho(t, []string{ + "exec", "--exec-profile", "fast", "--trace", tracePath, "hello", + }) + if exitCode != exitSuccess { + t.Fatalf("expected exit %d, got %d: %s", exitSuccess, exitCode, stderr) + } + raw, err := os.ReadFile(tracePath) + if err != nil { + t.Fatalf("read trace: %v", err) + } + if !strings.Contains(string(raw), `"profile":"fast"`) { + t.Fatalf("trace must record the selected profile, got %s", raw) + } +} + +// An explicit --spec-reasoning-effort governs the spec draft's effort, so the +// escalation effort-restore must not arm there even when the profile filled +// the (unused) main effort. +func TestSpecProfileEffortFilled(t *testing.T) { + if !specProfileEffortFilled(true, "") { + t.Fatal("no explicit spec effort: the profile's fill governs, restore must arm") + } + if specProfileEffortFilled(true, "high") { + t.Fatal("an explicit spec effort must disarm the effort restore") + } + if specProfileEffortFilled(false, "") { + t.Fatal("nothing filled, nothing to restore") + } +} + +// The trace label deliberately records the SELECTED profile, balanced +// included, so captures are self-describing. Run behavior stays identical +// (balanced fills nothing and arms no policy); the label is the one intended +// difference in the opt-in trace artifact. +func TestRunExecTraceRecordsBalancedSelection(t *testing.T) { + tracePath := filepath.Join(t.TempDir(), "trace.ndjson") + exitCode, _, stderr := runExecWithEcho(t, []string{ + "exec", "--exec-profile", "balanced", "--trace", tracePath, "hello", + }) + if exitCode != exitSuccess { + t.Fatalf("expected exit %d, got %d: %s", exitSuccess, exitCode, stderr) + } + raw, err := os.ReadFile(tracePath) + if err != nil { + t.Fatalf("read trace: %v", err) + } + if !strings.Contains(string(raw), `"profile":"balanced"`) { + t.Fatalf("trace must record the balanced selection, got %s", raw) + } +} + +func TestRunExecUnknownExecProfileIsUsageError(t *testing.T) { + exitCode, _, stderr := runExecWithEcho(t, []string{ + "exec", "--exec-profile", "turbo", "hello", + }) + if exitCode != exitUsage { + t.Fatalf("expected usage exit %d, got %d", exitUsage, exitCode) + } + if !strings.Contains(stderr, "unknown execution profile") { + t.Fatalf("expected an unknown-profile usage error, got %q", stderr) + } +} diff --git a/internal/cli/exec_spec.go b/internal/cli/exec_spec.go index 2fe910242..f049417f7 100644 --- a/internal/cli/exec_spec.go +++ b/internal/cli/exec_spec.go @@ -46,6 +46,11 @@ type execSpecDraftRun struct { reasoningEffort string specPermissionMode agent.PermissionMode notifier *notify.Notifier + // profilePolicy carries the selected execution profile's escalation policy + // (nil when none): the profile displaces resolved.MaxTurns before this + // path branches off, so the spec-draft run needs the same safety net as + // the main run. + profilePolicy *agent.ProfilePolicy } type execSpecDraftInfo struct { @@ -119,6 +124,7 @@ func runExecSpecDraft(run execSpecDraftRun) int { ProviderName: run.resolved.Provider.Name, Model: run.resolved.Provider.Model, ReasoningEffort: run.reasoningEffort, + Profile: run.profilePolicy, Cwd: run.workspaceRoot, SystemPrompt: specmode.DraftSystemPrompt, Images: run.images, diff --git a/internal/cli/exec_test.go b/internal/cli/exec_test.go index a34e54e73..a277d553b 100644 --- a/internal/cli/exec_test.go +++ b/internal/cli/exec_test.go @@ -549,10 +549,14 @@ func TestExecMemberAutoToolListIncludesMutators(t *testing.T) { } func TestRunExecAcceptsLegacyModelProfileFlags(t *testing.T) { + // --profile (legacy, inert model profile) and --exec-profile (execution + // profile) are distinct flags and must coexist on one invocation. exitCode, stdout, stderr := runExecWithEcho(t, []string{ "exec", "--profile", "fast", + "--exec-profile", + "balanced", "--reasoning-effort", "low", "hello", diff --git a/internal/execprofile/profile.go b/internal/execprofile/profile.go new file mode 100644 index 000000000..c162553c4 --- /dev/null +++ b/internal/execprofile/profile.go @@ -0,0 +1,153 @@ +// Package execprofile defines named execution profiles: bundles of loop-posture +// knobs (turn budget, reasoning effort, self-correction, escalation triggers) +// that tune how hard the agent loop tries, independent of which model runs. +// Profiles COMPOSE with --mode: a mode answers "what runs" (model, tools), +// a profile answers "how hard the loop tries". Selection precedence is +// explicit flag > --mode > profile, enforced by the callers applying profiles +// last with the same fill-only-if-unset rule modes use. +// +// The catalog is deliberately tiny and value-stable: balanced is the empty +// profile (selecting it leaves the run's options, budget, and loop behavior +// identical to an unflagged run — asserted by test; only the opt-in trace and +// bench artifacts record the selected profile name, by design, so captures are +// self-describing), fast trades turn budget and effort for latency with a +// one-shot escalation back to the displaced posture as the safety net, and +// thorough raises the budget and arms full self-correction. +package execprofile + +import ( + "sort" + "strings" + + "github.com/Gitlawb/zero/internal/agent" + "github.com/Gitlawb/zero/internal/sandbox" +) + +// Profile is a named execution posture. Zero-valued fields inherit the run's +// existing value (flag, mode, config, or built-in default) — a profile only +// ever fills knobs the caller left unset, except MaxTurns which REPLACES the +// resolved budget (that displacement is what escalation restores). +type Profile struct { + // Name identifies the profile in traces, help text, and diagnostics. + Name string + // MaxTurns replaces the resolved per-run tool-turn budget when > 0 and the + // user did not pass an explicit --max-turns. The displaced resolved value + // becomes the escalation target (see Policy). + MaxTurns int + // ReasoningEffort fills the run's effort when the user left it unset. It + // still flows through the model-supported gating at the call site, so an + // unsupported level degrades exactly like an explicit flag would. + ReasoningEffort string + // SelfCorrect arms the post-edit verify-and-correct loop. Profiles can only + // turn it ON (the flag is presence-only, so false is indistinguishable from + // unset and must never override an explicit opt-in). + SelfCorrect bool + + // Escalation triggers. All zero means the profile never escalates and + // Policy returns nil (the loop stays byte-identical to a nil-profile run). + // Targets are NOT part of the catalog: escalation restores the values the + // profile displaced at selection time, never invented ones, so targets are + // stamped by Policy from what the caller measured. + EscalateOnToolFailureStreak int + EscalateOnCompletionUncertain int + EscalateOnSelfCorrectFailure bool + EscalateOnRiskyMutation sandbox.RiskLevel +} + +// The catalog. Values marked provisional are floors/ceilings derived from the +// Phase 0 baseline's read-class evidence (successful nav runs used single-digit +// turns; the mutating classes produced no successful samples to tune from) and +// are expected to be re-tuned from the post-oracle-fix re-capture. The +// escalation safety net is what makes shipping provisional Fast values safe: +// a run that hits trouble gets its displaced budget back mid-run. +var ( + // Balanced is the default posture: empty on purpose. Selecting it changes + // nothing — the no-regression invariant of the whole feature. + Balanced = Profile{Name: "balanced"} + + // Fast starts cheap (30 turns, low effort) and arms every escalation + // trigger so a struggling run restores its displaced posture: two + // same-tool retriable failures in a row, a second uncertain completion + // evaluation, a failing self-correction cycle, or a critical-risk + // mutation. The risk threshold is deliberately Critical, not High: the + // sandbox classifies EVERY shell command as at least high risk before any + // command analysis, so a High trigger would fire on the first `go test` + // of virtually any coding task and spend the one-shot on turn one. + // Critical marks the genuinely scary categories (destructive commands, + // piped installers, out-of-workspace writes, network mutations). + Fast = Profile{ + Name: "fast", + MaxTurns: 30, + ReasoningEffort: "low", + EscalateOnToolFailureStreak: 2, + EscalateOnCompletionUncertain: 2, + EscalateOnSelfCorrectFailure: true, + EscalateOnRiskyMutation: sandbox.RiskCritical, + } + + // Thorough doubles the default budget, asks for high effort, and arms the + // full post-edit verify-and-correct loop. Already the maximum posture, so + // it has nothing to escalate to. + Thorough = Profile{ + Name: "thorough", + MaxTurns: 160, + ReasoningEffort: "high", + SelfCorrect: true, + } +) + +var catalog = map[string]Profile{ + Balanced.Name: Balanced, + Fast.Name: Fast, + Thorough.Name: Thorough, +} + +// Lookup resolves a profile by name, case-insensitively and ignoring +// surrounding whitespace. +func Lookup(name string) (Profile, bool) { + profile, ok := catalog[strings.ToLower(strings.TrimSpace(name))] + return profile, ok +} + +// Names returns the catalog's profile names, sorted, for usage errors and help. +func Names() []string { + names := make([]string, 0, len(catalog)) + for name := range catalog { + names = append(names, name) + } + sort.Strings(names) + return names +} + +// Policy projects the profile into the loop-facing ProfilePolicy. +// +// displacedMaxTurns is the turn budget this profile displaced at selection time +// (what the run would have used without it) and becomes the escalation's +// restore target; pass 0 when the profile did not displace the budget (e.g. +// the user pinned --max-turns explicitly) so escalation leaves the ceiling +// untouched. effortFilled reports whether the profile actually filled the +// run's reasoning effort (it backs off when the user set one explicitly); the +// displaced effort is then "" (the provider default) by construction, which +// the escalation restores via RestoreDefaultEffort — a plain "" target would +// mean "leave untouched". +// +// Profiles with no armed triggers return nil: the loop treats a nil policy as +// "no profile" (no observation, no counters), which keeps balanced and +// thorough runs byte-identical to the same knob values set by hand. +func (p Profile) Policy(displacedMaxTurns int, effortFilled bool) *agent.ProfilePolicy { + if p.EscalateOnToolFailureStreak == 0 && p.EscalateOnCompletionUncertain == 0 && + !p.EscalateOnSelfCorrectFailure && p.EscalateOnRiskyMutation == "" { + return nil + } + return &agent.ProfilePolicy{ + Name: p.Name, + Escalate: &agent.PostureEscalation{ + MaxTurns: displacedMaxTurns, + RestoreDefaultEffort: effortFilled, + OnToolFailureStreak: p.EscalateOnToolFailureStreak, + OnCompletionUncertain: p.EscalateOnCompletionUncertain, + OnSelfCorrectFailure: p.EscalateOnSelfCorrectFailure, + OnRiskyMutation: p.EscalateOnRiskyMutation, + }, + } +} diff --git a/internal/execprofile/profile_test.go b/internal/execprofile/profile_test.go new file mode 100644 index 000000000..3a5730192 --- /dev/null +++ b/internal/execprofile/profile_test.go @@ -0,0 +1,131 @@ +package execprofile + +import ( + "reflect" + "testing" + + "github.com/Gitlawb/zero/internal/sandbox" +) + +// The no-regression invariant of the whole feature: balanced must be the empty +// profile (nothing but a name), so selecting it cannot change a run. +func TestBalancedProfileIsEmpty(t *testing.T) { + profile, ok := Lookup("balanced") + if !ok { + t.Fatal("balanced must exist in the catalog") + } + if profile.Name != "balanced" { + t.Fatalf("Name = %q, want %q", profile.Name, "balanced") + } + profile.Name = "" + if !reflect.DeepEqual(profile, Profile{}) { + t.Fatalf("balanced must be zero-valued apart from its name, got %+v", profile) + } + if policy := Balanced.Policy(80, false); policy != nil { + t.Fatalf("balanced Policy must be nil (byte-identical loop), got %+v", policy) + } +} + +func TestLookupIsCaseAndSpaceInsensitive(t *testing.T) { + for _, name := range []string{"fast", "Fast", " FAST "} { + if _, ok := Lookup(name); !ok { + t.Fatalf("Lookup(%q) should resolve", name) + } + } + if _, ok := Lookup("turbo"); ok { + t.Fatal("Lookup(turbo) should not resolve") + } +} + +func TestNamesAreSorted(t *testing.T) { + want := []string{"balanced", "fast", "thorough"} + if got := Names(); !reflect.DeepEqual(got, want) { + t.Fatalf("Names() = %v, want %v", got, want) + } +} + +// Escalation targets must be the displaced values only — the catalog carries +// triggers, the call site supplies what was displaced, and nothing else may +// appear in the escalation. +func TestFastPolicyTargetsDisplacedValuesOnly(t *testing.T) { + policy := Fast.Policy(80, true) + if policy == nil { + t.Fatal("fast must arm an escalation policy") + } + if policy.Name != "fast" { + t.Fatalf("policy.Name = %q, want fast", policy.Name) + } + esc := policy.Escalate + if esc == nil { + t.Fatal("fast policy must carry an escalation") + } + if esc.MaxTurns != 80 { + t.Fatalf("Escalate.MaxTurns = %d, want the displaced 80", esc.MaxTurns) + } + if esc.ReasoningEffort != "" { + t.Fatalf("Escalate.ReasoningEffort = %q, want empty (displaced effort is the provider default by construction)", esc.ReasoningEffort) + } + if !esc.RestoreDefaultEffort { + t.Fatal("effort was profile-filled, so escalation must restore the provider default") + } + if esc.RestoreCompletionGate { + t.Fatal("fast does not touch the completion gate, so escalation must not either") + } + if esc.OnToolFailureStreak != Fast.EscalateOnToolFailureStreak || + esc.OnCompletionUncertain != Fast.EscalateOnCompletionUncertain || + esc.OnSelfCorrectFailure != Fast.EscalateOnSelfCorrectFailure || + esc.OnRiskyMutation != Fast.EscalateOnRiskyMutation { + t.Fatalf("escalation triggers must mirror the catalog, got %+v", esc) + } +} + +// The effort restore arms only when the profile actually filled the effort: +// with an explicit user effort the profile backed off, so escalation must not +// clear the user's choice. +func TestFastPolicyExplicitEffortIsNeverRestored(t *testing.T) { + policy := Fast.Policy(80, false) + if policy == nil || policy.Escalate == nil { + t.Fatal("fast must still arm its triggers") + } + if policy.Escalate.RestoreDefaultEffort { + t.Fatal("the profile did not fill the effort, so escalation must leave it alone") + } +} + +// A profile that did not displace the budget (explicit --max-turns pinned it) +// must not let escalation move the ceiling at all. +func TestFastPolicyZeroDisplacedLeavesCeilingUntouched(t *testing.T) { + policy := Fast.Policy(0, false) + if policy == nil || policy.Escalate == nil { + t.Fatal("fast must still arm its triggers with a zero displaced budget") + } + if policy.Escalate.MaxTurns != 0 { + t.Fatalf("Escalate.MaxTurns = %d, want 0 (no displaced value to restore)", policy.Escalate.MaxTurns) + } +} + +// Thorough is already the maximum posture: no triggers, so no policy — the +// loop must stay byte-identical to the same knobs set by hand. +func TestThoroughPolicyIsNil(t *testing.T) { + if policy := Thorough.Policy(80, true); policy != nil { + t.Fatalf("thorough Policy must be nil, got %+v", policy) + } +} + +// Pin the catalog's provisional values so a retune is a deliberate, +// test-visible diff (these floors came from the Phase 0 baseline's read-class +// evidence and are expected to move after the post-oracle-fix re-capture). +func TestCatalogProvisionalValues(t *testing.T) { + if Fast.MaxTurns != 30 || Fast.ReasoningEffort != "low" || Fast.SelfCorrect { + t.Fatalf("fast knobs changed: %+v", Fast) + } + // Critical, not High: the sandbox classifies every shell command as at + // least high risk, so a High trigger would spend the one-shot escalation + // on the first benign `go test` of any coding task. + if Fast.EscalateOnRiskyMutation != sandbox.RiskCritical { + t.Fatalf("fast risky-mutation trigger = %q, want %q", Fast.EscalateOnRiskyMutation, sandbox.RiskCritical) + } + if Thorough.MaxTurns != 160 || Thorough.ReasoningEffort != "high" || !Thorough.SelfCorrect { + t.Fatalf("thorough knobs changed: %+v", Thorough) + } +} diff --git a/internal/perfbench/taskbench.go b/internal/perfbench/taskbench.go index f3211afa2..cfcab1a86 100644 --- a/internal/perfbench/taskbench.go +++ b/internal/perfbench/taskbench.go @@ -85,6 +85,10 @@ type RunContext struct { Model string Mode string SelfCorrect bool + // ExecProfile, when non-empty, is forwarded to the agent invocation as + // --exec-profile (turn benchmark runner; the task benchmark leaves it + // empty). + ExecProfile string } // TaskRunner runs a single benchmark task and returns its outcome. diff --git a/internal/perfbench/turn_bench.go b/internal/perfbench/turn_bench.go index 2594d37b4..839c7362e 100644 --- a/internal/perfbench/turn_bench.go +++ b/internal/perfbench/turn_bench.go @@ -15,6 +15,7 @@ import ( "strings" "time" + "github.com/Gitlawb/zero/internal/execprofile" "github.com/Gitlawb/zero/internal/trace" ) @@ -52,7 +53,13 @@ import ( // attempted task errored. Accounting: an errored oracle task stays in its // tier denominator as a failure; an errored latency-only task counts in // latencyOnlyTasks and, as always, in no pass rate. -const TurnSchemaVersion = 4 +// +// v5 is ADDITIVE ONLY: execProfile records the execution profile the run was +// benchmarked under (omitted when none was selected), so profile A/B captures +// are self-describing and two reports can't be compared without noticing they +// ran different postures. No existing field changed shape or meaning; a v4 +// consumer reading a v5 report misses only the new field. +const TurnSchemaVersion = 5 // TurnRunner runs one benchmark task and reports its outcome plus the captured // per-turn trace. A non-nil Err means the run failed to execute (process crash); @@ -79,6 +86,10 @@ type TurnBenchConfig struct { Model string Mode string SelfCorrect bool + // ExecProfile, when non-empty, runs every task under the named execution + // profile (zero exec --exec-profile) and stamps it into the result so the + // report is self-describing for profile A/B comparisons. + ExecProfile string Version string Commit string // Iterations is how many times each task is run. The per-process `zero exec` @@ -151,6 +162,7 @@ type TurnBenchResult struct { Suite string `json:"suite"` Model string `json:"model"` Mode string `json:"mode,omitempty"` + ExecProfile string `json:"execProfile,omitempty"` SelfCorrect bool `json:"selfCorrect"` Version string `json:"version,omitempty"` Commit string `json:"commit,omitempty"` @@ -202,6 +214,19 @@ func RunTurnBench(ctx context.Context, set TaskSet, cfg TurnBenchConfig) (TurnBe if cfg.Runner == nil { return TurnBenchResult{}, errors.New("turn benchmark requires a runner") } + // Canonicalize the profile once at the boundary so every consumer — the + // child exec args, the stamped result, the summary — carries the same + // catalog name regardless of the caller's casing/whitespace, and a direct + // library caller cannot slip an unknown name into a report the way the CLI + // (which validates at parse time) cannot. + benchProfile := strings.TrimSpace(cfg.ExecProfile) + if benchProfile != "" { + profile, ok := execprofile.Lookup(benchProfile) + if !ok { + return TurnBenchResult{}, fmt.Errorf("unknown execution profile %q (valid: %s)", cfg.ExecProfile, strings.Join(execprofile.Names(), ", ")) + } + benchProfile = profile.Name + } iterations := cfg.Iterations if iterations < 1 { iterations = 1 @@ -210,7 +235,7 @@ func RunTurnBench(ctx context.Context, set TaskSet, cfg TurnBenchConfig) (TurnBe if now == nil { now = time.Now } - rc := RunContext{Model: cfg.Model, Mode: cfg.Mode, SelfCorrect: cfg.SelfCorrect} + rc := RunContext{Model: cfg.Model, Mode: cfg.Mode, SelfCorrect: cfg.SelfCorrect, ExecProfile: benchProfile} perSpanSamples := map[string][]float64{} classWalls := map[string][]float64{} @@ -240,6 +265,7 @@ func RunTurnBench(ctx context.Context, set TaskSet, cfg TurnBenchConfig) (TurnBe Suite: strings.TrimSpace(set.ID), Model: strings.TrimSpace(cfg.Model), Mode: strings.TrimSpace(cfg.Mode), + ExecProfile: benchProfile, SelfCorrect: cfg.SelfCorrect, Version: strings.TrimSpace(cfg.Version), Commit: strings.TrimSpace(cfg.Commit), @@ -496,6 +522,9 @@ func FormatTurnBenchSummary(result TurnBenchResult) string { if result.Mode != "" { lines = append(lines, "mode: "+result.Mode) } + if result.ExecProfile != "" { + lines = append(lines, "exec-profile: "+result.ExecProfile) + } if len(result.TopLatency) > 0 { lines = append(lines, "top latency sources:") for _, src := range result.TopLatency { @@ -771,6 +800,9 @@ func buildTurnExecArgs(task BenchTask, rc RunContext, tracePath string, extraArg if rc.SelfCorrect { args = append(args, "--self-correct") } + if profile := strings.TrimSpace(rc.ExecProfile); profile != "" { + args = append(args, "--exec-profile", profile) + } args = append(args, extraArgs...) args = append(args, task.Prompt) return args diff --git a/internal/perfbench/turn_bench_test.go b/internal/perfbench/turn_bench_test.go index 13be90a94..b123af027 100644 --- a/internal/perfbench/turn_bench_test.go +++ b/internal/perfbench/turn_bench_test.go @@ -536,10 +536,12 @@ func TestRunTurnBenchBuildOnlyMechanism(t *testing.T) { // and nav answer-oracles moved into correctnessPassRate). v4 adds tasksErrored: // tasks whose every iteration died before the agent produced a run are now // first-class in the report instead of visible only in warnings, so a -// spawn-broken run cannot print a clean-looking summary or exit 0. +// spawn-broken run cannot print a clean-looking summary or exit 0. v5 is +// additive only: execProfile stamps the execution profile the run was +// benchmarked under so profile A/B reports are self-describing. func TestTurnSchemaVersion(t *testing.T) { - if TurnSchemaVersion != 4 { - t.Fatalf("TurnSchemaVersion = %d, want 4", TurnSchemaVersion) + if TurnSchemaVersion != 5 { + t.Fatalf("TurnSchemaVersion = %d, want 5", TurnSchemaVersion) } } @@ -1161,3 +1163,95 @@ func TestRunTurnBenchPartialErrorStillCounts(t *testing.T) { t.Fatalf("TasksAttempted=%d, want 2", result.TasksAttempted) } } + +// The exec-profile passthrough: the profile must reach every task's zero exec +// invocation as --exec-profile (with the prompt staying last) and stay out of +// the args entirely when unset. +func TestBuildTurnExecArgsIncludesExecProfile(t *testing.T) { + task := BenchTask{ID: "t", Prompt: "do the thing"} + args := buildTurnExecArgs(task, RunContext{Model: "m", ExecProfile: "fast"}, "trace.ndjson", nil) + found := false + for i := 0; i < len(args)-1; i++ { + if args[i] == "--exec-profile" && args[i+1] == "fast" { + found = true + break + } + } + if !found { + t.Fatalf("args must carry --exec-profile fast, got %v", args) + } + if args[len(args)-1] != "do the thing" { + t.Fatalf("prompt must stay the last argument, got %v", args) + } + + args = buildTurnExecArgs(task, RunContext{Model: "m"}, "trace.ndjson", nil) + for _, arg := range args { + if arg == "--exec-profile" { + t.Fatalf("no profile configured, but args carry --exec-profile: %v", args) + } + } +} + +// The configured profile must reach the runner's RunContext and be stamped +// into the result, so a profile A/B report is self-describing. The boundary +// canonicalizes (case/whitespace) so equivalent postures always carry the same +// label, and rejects unknown names so a direct library caller cannot slip an +// unvalidated profile into a report the way the CLI (parse-time check) cannot. +func TestRunTurnBenchStampsExecProfile(t *testing.T) { + set := TaskSet{ + ID: "profile-suite", + Tasks: []BenchTask{{ID: "t1", Class: "longproc", Prompt: "p"}}, + } + var gotProfile string + cfg := TurnBenchConfig{ + Model: "fake-model", + ExecProfile: " FAST ", + Iterations: 1, + Runner: func(_ context.Context, _ BenchTask, rc RunContext) TurnTaskOutcome { + gotProfile = rc.ExecProfile + return TurnTaskOutcome{Passed: true, WallMs: 10, Trace: cannedTrace(10, 1, 100)} + }, + Now: func() time.Time { return time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) }, + } + result, err := RunTurnBench(context.Background(), set, cfg) + if err != nil { + t.Fatalf("RunTurnBench: %v", err) + } + if gotProfile != "fast" { + t.Fatalf("runner RunContext.ExecProfile = %q, want the canonical fast", gotProfile) + } + if result.ExecProfile != "fast" { + t.Fatalf("result.ExecProfile = %q, want the canonical fast", result.ExecProfile) + } + raw, err := json.Marshal(result) + if err != nil { + t.Fatalf("marshal result: %v", err) + } + if !strings.Contains(string(raw), `"execProfile":"fast"`) { + t.Fatal("published JSON must carry execProfile") + } +} + +func TestRunTurnBenchRejectsUnknownExecProfile(t *testing.T) { + set := TaskSet{ + ID: "profile-suite", + Tasks: []BenchTask{{ID: "t1", Class: "longproc", Prompt: "p"}}, + } + ran := false + cfg := TurnBenchConfig{ + Model: "fake-model", + ExecProfile: "blanced", + Iterations: 1, + Runner: func(context.Context, BenchTask, RunContext) TurnTaskOutcome { + ran = true + return TurnTaskOutcome{Passed: true, WallMs: 10, Trace: cannedTrace(10, 1, 100)} + }, + } + _, err := RunTurnBench(context.Background(), set, cfg) + if err == nil || !strings.Contains(err.Error(), "unknown execution profile") { + t.Fatalf("err = %v, want an unknown-profile rejection", err) + } + if ran { + t.Fatal("nothing may run under an unknown profile") + } +} diff --git a/internal/tui/command_center.go b/internal/tui/command_center.go index 2f5175a09..638bc2bef 100644 --- a/internal/tui/command_center.go +++ b/internal/tui/command_center.go @@ -464,13 +464,15 @@ func (m model) handleModelCommand(args string) (model, string) { // dedupe against the entry just persisted here, showing the same switch twice. config.RecentModelEntry{Provider: m.providerName, Model: target.modelID}, ) - resetEffort := false - if m.reasoningEffort != "" && !reasoningEffortAllowed(target.reasoningEfforts, m.reasoningEffort) { - // Drop an unsupported carry-over preference and fall back to the - // model's effective default for the new model. - m.reasoningEffort = "" - resetEffort = true - } + // Drop a known-unsupported preference, void any profile bookkeeping the + // drop erased, and re-derive an active profile's per-model effort fill. + // The ring is authoritative only for catalog-resolved targets + // (target.entry non-nil); live-discovered/custom targets carry no support + // knowledge, so an explicit preference survives onto them. resetEffort + // reports a dropped preference nothing refilled (shown as a reset to + // auto). + var resetEffort bool + m, resetEffort = m.reconcileEffortForModelSwitch(target.reasoningEfforts, target.entry != nil) effortLine := "effort: " + m.effortDisplay() if resetEffort { // Preference was dropped: show "auto" (model default applies), not a @@ -551,6 +553,13 @@ func (m model) switchProviderModel(providerName, modelID string) (model, string, m.providerProfile = target m.providerName = target.Name m.modelName = target.Model + // An active profile's effort fill is per-model: re-derive it for the + // destination, exactly like handleModelCommand does. No generic + // unsupported-drop here: cross-provider targets are often custom models + // the catalog cannot vouch for either way, so an explicit preference is + // carried (pre-existing behavior) while the profile's own fill stays + // conservative — it only ever applies where support is known. + m = m.reconcileProfileAfterModelSwitch(m.availableReasoningEfforts()) // Record the outgoing pair too — see the matching comment in // handleModelCommand for why (keeps the session's starting model from // silently dropping out of "Recent" on the first switch away from it). diff --git a/internal/tui/commands.go b/internal/tui/commands.go index db1fad80f..57e091893 100644 --- a/internal/tui/commands.go +++ b/internal/tui/commands.go @@ -41,6 +41,7 @@ const ( commandAddDir commandSelfCorrect commandTurns + commandProfile commandRetry commandEdit commandCopy @@ -286,6 +287,13 @@ var commandDefinitions = []commandDefinition{ description: "Show or set the per-run tool-turn budget for this session (raise it for long multi-step tasks).", kind: commandTurns, }, + { + name: "/profile", + usage: "/profile [status|balanced|fast|thorough]", + group: commandGroupSession, + description: "Show or switch the execution profile for the next run (loop posture: turn budget, effort, self-correction, escalation; model selection is unchanged).", + kind: commandProfile, + }, { name: "/retry", usage: "/retry", diff --git a/internal/tui/model.go b/internal/tui/model.go index b47be5957..b44d7be8d 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -128,20 +128,36 @@ type model struct { permissionMode agent.PermissionMode selfCorrectTests bool reasoningEffort modelregistry.ReasoningEffort - responseStyle string - keyBindings keyBindings - themeMode themeMode // palette preference: auto (default), dark, light - hasDarkBg bool // last terminal background-detection result (auto mode) - userAgent string - compactRequests int - compactInFlight bool - compactFrame int - lastCompactResult *CompactResult - lastCompactError string - unpricedRequests int - unpricedTokens int - lastUsage usage.Normalized - lastUsageSeen bool + // Active execution profile (set by /profile; applies to the NEXT run). + // The displaced/applied pairs let a switch or /profile balanced restore + // exactly what the profile replaced while leaving later manual overrides + // (/turns, Ctrl+T, /selfcorrect) alone: each knob is only reverted when it + // still holds the value the profile applied. + execProfileName string + execProfileDisplacedMaxTurns int + execProfileAppliedMaxTurns int + execProfileAppliedEffort modelregistry.ReasoningEffort + execProfileArmedSelfCorrect bool + // The touched bits record explicit user choices made while a profile is + // active (/turns, /effort, Ctrl+T, /selfcorrect); a touched knob is never + // reverted, even when its value coincides with what the profile applied. + execProfileTurnsTouched bool + execProfileEffortTouched bool + execProfileSelfCorrectTouched bool + responseStyle string + keyBindings keyBindings + themeMode themeMode // palette preference: auto (default), dark, light + hasDarkBg bool // last terminal background-detection result (auto mode) + userAgent string + compactRequests int + compactInFlight bool + compactFrame int + lastCompactResult *CompactResult + lastCompactError string + unpricedRequests int + unpricedTokens int + lastUsage usage.Normalized + lastUsageSeen bool // turnLatencySum / turnLatencyCount accumulate completed-run wall time so // /context can show a rolling average turn latency (the "is it slow?" signal). // Reset by /new. @@ -4346,6 +4362,18 @@ func (m model) dispatchCommand(command parsedCommand) (tea.Model, tea.Cmd) { m, text = m.handleTurnsCommand(command.text) m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendSystem, text: text}) return m, nil + case commandProfile: + // Same idle-session rule as /turns: switching the profile mutates the + // turn budget (and its ZERO_MAX_TURNS propagation), so a change needs + // an idle session; bare /profile (status) is always allowed. + if m.pending && strings.TrimSpace(command.text) != "" && !strings.EqualFold(strings.TrimSpace(command.text), "status") { + m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendSystem, text: "Profile\nFinish or stop the current run before switching the execution profile."}) + return m, nil + } + text := "" + m, text = m.handleProfileCommand(command.text) + m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendSystem, text: text}) + return m, nil case commandTheme: // Bare `/theme` opens the popup picker (live preview on move, apply on // Enter), matching /model and /effort. An explicit `/theme auto|dark|light` diff --git a/internal/tui/profile_command_test.go b/internal/tui/profile_command_test.go new file mode 100644 index 000000000..f9884c7f9 --- /dev/null +++ b/internal/tui/profile_command_test.go @@ -0,0 +1,432 @@ +package tui + +import ( + "context" + "os" + "strings" + "testing" + + "github.com/Gitlawb/zero/internal/config" + "github.com/Gitlawb/zero/internal/modelregistry" + "github.com/Gitlawb/zero/internal/zeroruntime" +) + +// profileSwitchModel builds a session on a model whose catalog entry supports +// the fast profile's "low" effort, with a second saved provider whose custom +// model has no catalog entry (so no effort ring) — the two directions the +// model-switch reconciliation must handle. +func profileSwitchModel(t *testing.T) model { + t.Helper() + return newModel(context.Background(), Options{ + ProviderName: "anthropic", + ModelName: "claude-sonnet-4.5", + Provider: &fakeProvider{}, + ProviderProfile: config.ProviderProfile{Name: "anthropic", CatalogID: "anthropic", Model: "claude-sonnet-4.5", APIKey: "k"}, + SavedProviders: []config.ProviderProfile{ + {Name: "anthropic", CatalogID: "anthropic", Model: "claude-sonnet-4.5", APIKey: "k"}, + {Name: "ollama", CatalogID: "ollama", ProviderKind: config.ProviderKindOpenAICompatible, BaseURL: "http://localhost:11434/v1", Model: "kimi-k2.7-code:cloud"}, + }, + NewProvider: func(config.ProviderProfile) (zeroruntime.Provider, error) { + return &fakeProvider{}, nil + }, + }) +} + +// The profile's effort fill is per-model and must be re-derived on a model +// switch: dropped when the destination does not support the level, refilled +// when a later destination does, with the escalation's effort restore tracking +// whether the profile currently governs the effort. +func TestProfileEffortReconciledOnModelSwitch(t *testing.T) { + m := profileSwitchModel(t) + + m, _ = m.handleProfileCommand("fast") + if m.reasoningEffort != "low" || m.agentOptions.Profile == nil || !m.agentOptions.Profile.Escalate.RestoreDefaultEffort { + t.Fatalf("fast on a low-supporting model must fill low and arm the restore, got effort %q", m.reasoningEffort) + } + + // Supported -> unsupported: the profile-applied level must not survive + // onto a model with no effort ring. + m, text, ok, _ := m.switchProviderModel("ollama", "kimi-k2.7-code:cloud") + if !ok { + t.Fatalf("switch to ollama failed: %q", text) + } + if m.reasoningEffort != "" || m.execProfileAppliedEffort != "" { + t.Fatalf("effort = %q applied = %q, want both cleared on an unsupported destination", m.reasoningEffort, m.execProfileAppliedEffort) + } + if m.agentOptions.Profile.Escalate.RestoreDefaultEffort { + t.Fatal("the profile no longer governs the effort, so the restore must disarm") + } + if m.execProfileName != "fast" || m.agentOptions.MaxTurns != 30 { + t.Fatalf("profile must stay active across the switch: name %q turns %d", m.execProfileName, m.agentOptions.MaxTurns) + } + + // Unsupported -> supported: switching (back) to a supporting model must + // behave like selecting the profile there. + m, text, ok, _ = m.switchProviderModel("anthropic", "claude-sonnet-4.5") + if !ok { + t.Fatalf("switch back to anthropic failed: %q", text) + } + if m.reasoningEffort != "low" || m.execProfileAppliedEffort != "low" { + t.Fatalf("effort = %q applied = %q, want the profile's low refilled", m.reasoningEffort, m.execProfileAppliedEffort) + } + if !m.agentOptions.Profile.Escalate.RestoreDefaultEffort { + t.Fatal("the profile governs the effort again, so the restore must re-arm") + } +} + +// The direct /model path (reconcileEffortForModelSwitch, driven by the +// target's authoritative ring) drops a KNOWN-unsupported effort — the TUI run +// path forwards the effort unfiltered, so it must not survive. When that drop +// voids a choice made under an active profile, the profile's effort +// bookkeeping resets and the profile resumes governing: a later switch to a +// supporting ring fills the profile's level again. +func TestProfileEffortVoidedByModelSwitchDrop(t *testing.T) { + m := profileSwitchModel(t) + + m, _ = m.handleProfileCommand("fast") + m, _ = m.handleEffortCommand("high") + if m.reasoningEffort != "high" || !m.execProfileEffortTouched { + t.Fatalf("setup: effort %q touched %v, want an explicit high", m.reasoningEffort, m.execProfileEffortTouched) + } + + // Destination with an AUTHORITATIVE empty ring (a catalog model without + // effort controls): "high" is known-unsupported, the drop fires, and the + // voided choice resets the profile's bookkeeping. + m, reset := m.reconcileEffortForModelSwitch(nil, true) + if !reset { + t.Fatal("a dropped preference with nothing refilled must report the reset") + } + if m.reasoningEffort != "" { + t.Fatalf("effort = %q, a known-unsupported level must not survive", m.reasoningEffort) + } + if m.execProfileEffortTouched || m.execProfileAppliedEffort != "" { + t.Fatalf("voided choice must reset the bookkeeping: touched %v applied %q", m.execProfileEffortTouched, m.execProfileAppliedEffort) + } + if m.agentOptions.Profile.Escalate.RestoreDefaultEffort { + t.Fatal("no profile-governed effort on this model, so the restore must be disarmed") + } + if m.execProfileName != "fast" || m.agentOptions.MaxTurns != 30 { + t.Fatalf("profile must stay active: name %q turns %d", m.execProfileName, m.agentOptions.MaxTurns) + } + + // With the void cleared, a destination that supports the profile's level + // behaves like selecting the profile there: the fill returns. + ring := []modelregistry.ReasoningEffort{"low", "medium", "high"} + m, reset = m.reconcileEffortForModelSwitch(ring, true) + if reset { + t.Fatal("a refill is not a reset to auto") + } + if m.reasoningEffort != "low" || m.execProfileAppliedEffort != "low" { + t.Fatalf("effort = %q applied = %q, want the profile's low refilled", m.reasoningEffort, m.execProfileAppliedEffort) + } + if !m.agentOptions.Profile.Escalate.RestoreDefaultEffort { + t.Fatal("the profile governs the effort again, so the restore must re-arm") + } +} + +// A touched effort the destination DOES support survives the direct /model +// path untouched — the drop only fires for known-unsupported levels. +func TestProfileEffortTouchedSurvivesSupportedModelSwitch(t *testing.T) { + m := profileSwitchModel(t) + + m, _ = m.handleProfileCommand("fast") + m, _ = m.handleEffortCommand("high") + m, reset := m.reconcileEffortForModelSwitch([]modelregistry.ReasoningEffort{"low", "medium", "high"}, true) + if reset { + t.Fatal("nothing was dropped, so no reset") + } + if m.reasoningEffort != "high" || !m.execProfileEffortTouched { + t.Fatalf("effort = %q touched %v, an explicit choice the destination supports must survive", m.reasoningEffort, m.execProfileEffortTouched) + } + if m.agentOptions.Profile.Escalate.RestoreDefaultEffort { + t.Fatal("an explicit choice keeps the restore disarmed") + } +} + +// An UNKNOWN ring (live-discovered/custom target with no catalog entry) is +// not evidence of unsupported: an explicit preference survives, exactly as it +// does on the cross-provider picker path, while the profile's own fill still +// retreats to models where support is known. +func TestProfileEffortUnknownRingPreservesExplicitChoice(t *testing.T) { + m := profileSwitchModel(t) + + m, _ = m.handleProfileCommand("fast") + m, _ = m.handleEffortCommand("high") + m, reset := m.reconcileEffortForModelSwitch(nil, false) + if reset { + t.Fatal("an unknown ring must not report a reset") + } + if m.reasoningEffort != "high" || !m.execProfileEffortTouched { + t.Fatalf("effort = %q touched %v, an explicit choice must survive an unknown ring", m.reasoningEffort, m.execProfileEffortTouched) + } + + // Untouched profile fill on an unknown ring: the fill retreats (the + // profile only governs where support is known), with clean bookkeeping. + m2 := profileSwitchModel(t) + m2, _ = m2.handleProfileCommand("fast") + m2, reset = m2.reconcileEffortForModelSwitch(nil, false) + if reset { + t.Fatal("a profile-fill retreat is not an unsupported-preference reset") + } + if m2.reasoningEffort != "" || m2.execProfileAppliedEffort != "" { + t.Fatalf("effort = %q applied = %q, the profile fill must retreat on an unknown ring", m2.reasoningEffort, m2.execProfileAppliedEffort) + } + if m2.agentOptions.Profile.Escalate.RestoreDefaultEffort { + t.Fatal("no profile-governed effort, so the restore must be disarmed") + } +} + +// An explicitly touched effort is the user's choice: the reconciliation must +// leave it alone entirely across model switches. +func TestProfileEffortTouchedSurvivesModelSwitch(t *testing.T) { + m := profileSwitchModel(t) + + m, _ = m.handleProfileCommand("fast") + m, _ = m.handleEffortCommand("high") + m, text, ok, _ := m.switchProviderModel("ollama", "kimi-k2.7-code:cloud") + if !ok { + t.Fatalf("switch to ollama failed: %q", text) + } + if m.reasoningEffort != "high" { + t.Fatalf("effort = %q, an explicit choice must survive the switch untouched", m.reasoningEffort) + } + if m.agentOptions.Profile.Escalate.RestoreDefaultEffort { + t.Fatal("an explicit choice keeps the restore disarmed") + } +} + +func TestProfileCommandStatus(t *testing.T) { + _, out := model{}.handleProfileCommand("") + if !strings.Contains(out, "balanced (default)") { + t.Fatalf("bare /profile must report the balanced default, got %q", out) + } + _, out = model{}.handleProfileCommand("status") + if !strings.Contains(out, "balanced (default)") { + t.Fatalf("/profile status must report the balanced default, got %q", out) + } +} + +func TestProfileCommandUnknownValue(t *testing.T) { + got, out := model{}.handleProfileCommand("turbo") + if !strings.Contains(out, "Usage:") { + t.Fatalf("unknown profile must print usage, got %q", out) + } + if got.execProfileName != "" || got.agentOptions.Profile != nil { + t.Fatal("unknown profile must not change any state") + } +} + +// Fast applies to the next run: the turn budget drops, the escalation policy +// is armed with the DISPLACED budget as its restore target, and effort stays +// untouched when the session has no model-supported effort ring (model{}). +func TestProfileCommandSetsFastPosture(t *testing.T) { + m := model{} + m.agentOptions.MaxTurns = 80 + + got, out := m.handleProfileCommand("fast") + if got.agentOptions.MaxTurns != 30 { + t.Fatalf("MaxTurns = %d, want the fast profile's 30", got.agentOptions.MaxTurns) + } + if got.execProfileName != "fast" { + t.Fatalf("execProfileName = %q, want fast", got.execProfileName) + } + policy := got.agentOptions.Profile + if policy == nil || policy.Escalate == nil { + t.Fatal("fast must arm an escalation policy on agentOptions") + } + if policy.Escalate.MaxTurns != 80 { + t.Fatalf("Escalate.MaxTurns = %d, want the displaced 80", policy.Escalate.MaxTurns) + } + if got.reasoningEffort != "" { + t.Fatalf("effort = %q, must stay auto when the model exposes no effort ring", got.reasoningEffort) + } + if !strings.Contains(out, "fast") || !strings.Contains(out, "headless-only") { + t.Fatalf("status must name the profile and the TUI signal asymmetry, got %q", out) + } +} + +func TestProfileCommandThoroughArmsSelfCorrect(t *testing.T) { + m := model{} + m.agentOptions.MaxTurns = 80 + + got, _ := m.handleProfileCommand("thorough") + if got.agentOptions.MaxTurns != 160 { + t.Fatalf("MaxTurns = %d, want thorough's 160", got.agentOptions.MaxTurns) + } + if !got.selfCorrectTests { + t.Fatal("thorough must arm full self-correction") + } + if got.agentOptions.Profile != nil { + t.Fatal("thorough has no escalation triggers, so the loop policy must stay nil") + } +} + +func TestProfileCommandBalancedRestoresDisplaced(t *testing.T) { + m := model{} + m.agentOptions.MaxTurns = 80 + + m, _ = m.handleProfileCommand("fast") + got, _ := m.handleProfileCommand("balanced") + if got.agentOptions.MaxTurns != 80 { + t.Fatalf("MaxTurns = %d, want the displaced 80 restored", got.agentOptions.MaxTurns) + } + if got.execProfileName != "" || got.agentOptions.Profile != nil { + t.Fatal("balanced must clear the profile state and loop policy") + } +} + +// Profiles must not stack: switching fast -> thorough restores fast's +// displacement first, so thorough displaces the ORIGINAL budget and a later +// balanced lands back on it. +func TestProfileCommandSwitchDoesNotStack(t *testing.T) { + m := model{} + m.agentOptions.MaxTurns = 80 + + m, _ = m.handleProfileCommand("fast") + m, _ = m.handleProfileCommand("thorough") + if m.agentOptions.MaxTurns != 160 { + t.Fatalf("MaxTurns = %d, want thorough's 160", m.agentOptions.MaxTurns) + } + if m.execProfileDisplacedMaxTurns != 80 { + t.Fatalf("displaced = %d, want the original 80 (not fast's 30)", m.execProfileDisplacedMaxTurns) + } + m, _ = m.handleProfileCommand("balanced") + if m.agentOptions.MaxTurns != 80 { + t.Fatalf("MaxTurns = %d, want the original 80 restored", m.agentOptions.MaxTurns) + } +} + +// An explicit /turns while a profile is active is a pinned budget: it must +// disarm the escalation's turn target (mirroring headless exec, where an +// explicit --max-turns leaves nothing displaced) while the other triggers +// stay armed, and it must survive a later revert. +func TestProfileCommandTurnsPinDisarmsEscalationTarget(t *testing.T) { + m := model{} + m.agentOptions.MaxTurns = 80 + + m, _ = m.handleProfileCommand("fast") + m, _ = m.handleTurnsCommand("20") + if m.agentOptions.MaxTurns != 20 { + t.Fatalf("MaxTurns = %d, want the pinned 20", m.agentOptions.MaxTurns) + } + policy := m.agentOptions.Profile + if policy == nil || policy.Escalate == nil { + t.Fatal("the profile's other escalation triggers must stay armed") + } + if policy.Escalate.MaxTurns != 0 { + t.Fatalf("Escalate.MaxTurns = %d, want 0 (a pinned budget must never be raised by escalation)", policy.Escalate.MaxTurns) + } + if policy.Escalate.OnToolFailureStreak == 0 { + t.Fatal("disarming the turn target must not disarm the other triggers") + } + m, _ = m.handleProfileCommand("balanced") + if m.agentOptions.MaxTurns != 20 { + t.Fatalf("MaxTurns = %d, the pinned 20 must survive the revert", m.agentOptions.MaxTurns) + } +} + +// A manual override that COINCIDES with the profile's own value must still +// survive the revert: touched beats value equality. +func TestProfileCommandRevertLeavesCoincidingOverride(t *testing.T) { + m := model{} + m.agentOptions.MaxTurns = 80 + + m, _ = m.handleProfileCommand("fast") + m, _ = m.handleTurnsCommand("30") // explicit, happens to equal fast's 30 + m, _ = m.handleProfileCommand("balanced") + if m.agentOptions.MaxTurns != 30 { + t.Fatalf("MaxTurns = %d, the explicit /turns 30 must survive even though it equals fast's value", m.agentOptions.MaxTurns) + } + + // Same for self-correct: /selfcorrect off then on under thorough is an + // explicit opt-in, not the profile's arming. + m2 := model{} + m2.agentOptions.MaxTurns = 80 + m2, _ = m2.handleProfileCommand("thorough") + m2, _ = m2.handleSelfCorrectCommand("off") + m2, _ = m2.handleSelfCorrectCommand("on") + m2, _ = m2.handleProfileCommand("balanced") + if !m2.selfCorrectTests { + t.Fatal("an explicit /selfcorrect on must survive the revert even though thorough also armed it") + } +} + +// Reverting a profile whose displaced budget was zero (nothing was set before +// it) must clear ZERO_MAX_TURNS: SetMaxTurnsEnv ignores zero, so without an +// explicit unset, spawned sub-agents would keep the removed profile's budget. +func TestProfileCommandRevertFromZeroClearsMaxTurnsEnv(t *testing.T) { + t.Setenv(config.MaxTurnsEnv, "") + m := model{} // MaxTurns 0: the profile displaces nothing + + m, _ = m.handleProfileCommand("fast") + if got := os.Getenv(config.MaxTurnsEnv); got != "30" { + t.Fatalf("env after apply = %q, want the profile's 30", got) + } + m, _ = m.handleProfileCommand("balanced") + if m.agentOptions.MaxTurns != 0 { + t.Fatalf("MaxTurns = %d, want the displaced 0 restored", m.agentOptions.MaxTurns) + } + if got, present := os.LookupEnv(config.MaxTurnsEnv); present { + t.Fatalf("env after revert = %q (present), want the key removed", got) + } +} + +// An explicit effort choice after /profile must disarm the escalation's +// effort restore (the effort analog of the /turns pin): a mid-run escalation +// must never clear an effort the user pinned by hand. Needs a catalog model +// with an effort ring so the profile's fill actually applies. +func TestProfileCommandExplicitEffortDisarmsRestore(t *testing.T) { + m := model{modelName: "claude-sonnet-4.5"} + m.agentOptions.MaxTurns = 80 + + m, _ = m.handleProfileCommand("fast") + if m.reasoningEffort != "low" { + t.Fatalf("profile did not fill low effort for the test model: got %q", m.reasoningEffort) + } + policy := m.agentOptions.Profile + if policy == nil || policy.Escalate == nil || !policy.Escalate.RestoreDefaultEffort { + t.Fatal("profile-filled effort must arm the escalation effort restore") + } + + m, _ = m.handleEffortCommand("high") + if m.reasoningEffort != "high" { + t.Fatalf("effort = %q, want the explicit high", m.reasoningEffort) + } + policy = m.agentOptions.Profile + if policy == nil || policy.Escalate == nil { + t.Fatal("the escalation policy must stay armed") + } + if policy.Escalate.RestoreDefaultEffort { + t.Fatal("an explicit /effort must disarm the effort restore") + } + if policy.Escalate.MaxTurns != 80 { + t.Fatalf("Escalate.MaxTurns = %d, disarming effort must not touch the turn target", policy.Escalate.MaxTurns) + } + // And the explicit choice survives a later revert (touched bit). + m, _ = m.handleProfileCommand("balanced") + if m.reasoningEffort != "high" { + t.Fatalf("effort = %q, the explicit high must survive the revert", m.reasoningEffort) + } +} + +// A manual override made after selecting a profile survives the revert: the +// profile only restores knobs that still hold the value it applied. +func TestProfileCommandRevertLeavesManualOverride(t *testing.T) { + m := model{} + m.agentOptions.MaxTurns = 80 + + m, _ = m.handleProfileCommand("fast") + m, _ = m.handleTurnsCommand("100") + got, _ := m.handleProfileCommand("balanced") + if got.agentOptions.MaxTurns != 100 { + t.Fatalf("MaxTurns = %d, the user's explicit /turns 100 must survive the revert", got.agentOptions.MaxTurns) + } + // An explicit /selfcorrect choice armed BEFORE thorough must survive too. + m2 := model{selfCorrectTests: true} + m2.agentOptions.MaxTurns = 80 + m2, _ = m2.handleProfileCommand("thorough") + m2, _ = m2.handleProfileCommand("balanced") + if !m2.selfCorrectTests { + t.Fatal("an explicit self-correct opt-in must survive profile revert") + } +} diff --git a/internal/tui/session_controls.go b/internal/tui/session_controls.go index 4ce15fb5e..5d34ce216 100644 --- a/internal/tui/session_controls.go +++ b/internal/tui/session_controls.go @@ -3,6 +3,7 @@ package tui import ( "context" "fmt" + "os" "strconv" "strings" @@ -10,6 +11,7 @@ import ( "github.com/Gitlawb/zero/internal/agent" "github.com/Gitlawb/zero/internal/config" + "github.com/Gitlawb/zero/internal/execprofile" "github.com/Gitlawb/zero/internal/modelregistry" "github.com/Gitlawb/zero/internal/sessions" "github.com/Gitlawb/zero/internal/usage" @@ -61,6 +63,7 @@ func (m model) handleEffortCommand(args string) (model, string) { } if args == "auto" { m.reasoningEffort = "" + m = m.markProfileEffortTouched() return m, m.effortStatusCard("auto", "Reasoning effort selection will follow the active model/provider defaults.") } @@ -78,6 +81,7 @@ func (m model) handleEffortCommand(args string) (model, string) { } m.reasoningEffort = requested + m = m.markProfileEffortTouched() return m, m.effortStatusCard(string(requested), "Reasoning effort preference is stored for this TUI session.") } @@ -131,6 +135,104 @@ func (m model) effortText() string { }) } +// markProfileEffortTouched records an explicit effort choice made while a +// profile is active: the touched bit protects the choice from a later profile +// revert, and disarming the policy's RestoreDefaultEffort stops a mid-run +// escalation from clearing an effort the user pinned by hand. +func (m model) markProfileEffortTouched() model { + if m.execProfileName == "" { + return m + } + m.execProfileEffortTouched = true + return m.setProfileEffortRestore(false) +} + +// setProfileEffortRestore rewrites the armed policy's RestoreDefaultEffort on +// a clone (an in-flight run may hold the current pointer), like the /turns +// turn-target disarm. No-op when no escalation policy is armed or the flag +// already matches. +func (m model) setProfileEffortRestore(restore bool) model { + if m.agentOptions.Profile == nil || m.agentOptions.Profile.Escalate == nil || + m.agentOptions.Profile.Escalate.RestoreDefaultEffort == restore { + return m + } + policy := *m.agentOptions.Profile + escalate := *policy.Escalate + escalate.RestoreDefaultEffort = restore + policy.Escalate = &escalate + m.agentOptions.Profile = &policy + return m +} + +// reconcileEffortForModelSwitch applies the effort rules for a model switch. +// efforts is the destination's supported ring and ringKnown whether that ring +// is authoritative (a catalog entry, whose ring may be legitimately empty) or +// missing knowledge (a live-discovered/custom model the catalog cannot vouch +// for either way). +// +// First the pre-existing generic rule: a KNOWN-unsupported preference is +// dropped — the TUI run path forwards the effort to the provider unfiltered, +// so it must not survive the switch, for a profile-touched effort exactly as +// for a plain one. An UNKNOWN ring never drops an explicit preference: the +// model may well support it, matching the cross-provider picker's behavior +// for custom models. When a drop voids a choice living under an active +// profile (the profile's own fill or a user override of it), the profile's +// effort bookkeeping resets so a choice that no longer exists stops binding +// and no stale applied/restore state remains. +// +// Then the profile rule: an active, untouched profile re-derives its fill for +// the destination (conservative in both directions — the fill only ever +// applies where support is known). The second return reports whether an +// unsupported preference was dropped and nothing refilled (the caller's +// "(unsupported preference reset)" display case). +func (m model) reconcileEffortForModelSwitch(efforts []modelregistry.ReasoningEffort, ringKnown bool) (model, bool) { + dropped := false + if ringKnown && m.reasoningEffort != "" && !reasoningEffortAllowed(efforts, m.reasoningEffort) { + m.reasoningEffort = "" + dropped = true + if m.execProfileName != "" { + m.execProfileEffortTouched = false + m.execProfileAppliedEffort = "" + m = m.setProfileEffortRestore(false) + } + } + m = m.reconcileProfileAfterModelSwitch(efforts) + return m, dropped && m.reasoningEffort == "" +} + +// reconcileProfileAfterModelSwitch re-derives an active profile's effort fill +// for the model the session just switched to, whose supported ring is passed +// in. The fill is per-model, not a session preference: fast selected on a +// model without "low" fills nothing, and switching to a model that supports +// it should behave exactly like selecting the profile there (and the reverse +// switch must not keep sending a level the destination does not support). An +// explicitly touched effort is the user's choice and is never reconciled; the +// escalation's RestoreDefaultEffort tracks whether the profile currently +// governs the effort. +func (m model) reconcileProfileAfterModelSwitch(efforts []modelregistry.ReasoningEffort) model { + if m.execProfileName == "" || m.execProfileEffortTouched { + return m + } + profile, ok := execprofile.Lookup(m.execProfileName) + if !ok || profile.ReasoningEffort == "" { + return m + } + want := modelregistry.ReasoningEffort(profile.ReasoningEffort) + supported := reasoningEffortAllowed(efforts, want) + filled := m.execProfileAppliedEffort != "" + switch { + case supported && !filled && m.reasoningEffort == "": + m.reasoningEffort = want + m.execProfileAppliedEffort = want + m = m.setProfileEffortRestore(true) + case !supported && filled && m.reasoningEffort == m.execProfileAppliedEffort: + m.reasoningEffort = "" + m.execProfileAppliedEffort = "" + m = m.setProfileEffortRestore(false) + } + return m +} + func (m model) availableReasoningEfforts() []modelregistry.ReasoningEffort { if strings.TrimSpace(m.modelName) == "" { return nil @@ -161,6 +263,10 @@ func (m model) cycleReasoningEffort() (model, tea.Cmd) { if len(efforts) == 0 { return m, nil } + // Every branch below changes the effort; a cycle while a profile is active + // is an explicit user choice that must survive both a later profile revert + // and any mid-run escalation. + m = m.markProfileEffortTouched() if m.reasoningEffort == "" { m.reasoningEffort = efforts[0] return m, nil @@ -254,6 +360,12 @@ func (m model) handleSelfCorrectCommand(args string) (model, string) { default: return m, "Self-correct\nUsage: /selfcorrect [status|on|off|tests|full|lsp]" } + // An explicit choice while a profile is active must survive a later + // profile revert, even when it lands on the value the profile applied + // (e.g. /selfcorrect off then on under thorough). + if m.execProfileName != "" { + m.execProfileSelfCorrectTouched = true + } return m, m.selfCorrectText() } @@ -278,9 +390,146 @@ func (m model) handleTurnsCommand(args string) (model, string) { // Propagate the budget to spawned sub-agents / swarm members (which inherit the // environment) so a delegated task gets the same budget, not config.json's default. config.SetMaxTurnsEnv(n) + // An explicit /turns while a profile is active is a pinned budget: mirror + // headless exec's rule (an explicit --max-turns leaves nothing displaced) + // by disarming the escalation's turn target, so a mid-run escalation can + // never raise the budget the user just pinned. The other escalation + // triggers stay armed. Mark the knob touched so a later profile revert + // leaves this choice alone even if n coincides with the profile's value. + if m.execProfileName != "" { + m.execProfileTurnsTouched = true + if m.agentOptions.Profile != nil && m.agentOptions.Profile.Escalate != nil { + policy := *m.agentOptions.Profile + escalate := *policy.Escalate + escalate.MaxTurns = 0 + policy.Escalate = &escalate + m.agentOptions.Profile = &policy + } + } return m, m.turnsText() } +// handleProfileCommand shows or switches the session's execution profile: a +// loop-posture bundle (turn budget, reasoning effort, self-correction, +// escalation triggers) applied to the NEXT run, composing with the selected +// model rather than replacing it. Switching first restores whatever the +// previous profile displaced (profiles never stack), and balanced IS that +// restore: the empty profile, so selecting it just clears the posture. +func (m model) handleProfileCommand(args string) (model, string) { + args = strings.TrimSpace(args) + if args == "" || strings.EqualFold(args, "status") { + return m, m.profileText() + } + profile, ok := execprofile.Lookup(args) + if !ok { + return m, "Profile\nUsage: /profile [status|" + strings.Join(execprofile.Names(), "|") + "] — switch the next run's loop posture." + } + m = m.revertExecProfile() + if profile.Name == execprofile.Balanced.Name { + return m, m.profileText() + } + // Turn budget: displace the session's current budget. The displaced value + // becomes the escalation restore target, exactly like headless exec, and + // the new budget propagates to spawned sub-agents the same way /turns does. + displacedMaxTurns := 0 + if profile.MaxTurns > 0 { + displacedMaxTurns = m.agentOptions.MaxTurns + m.agentOptions.MaxTurns = profile.MaxTurns + m.execProfileAppliedMaxTurns = profile.MaxTurns + m.execProfileDisplacedMaxTurns = displacedMaxTurns + config.SetMaxTurnsEnv(profile.MaxTurns) + } + // Reasoning effort: fill only when the session is on auto AND the active + // model supports the profile's level, mirroring exec's supported-effort + // gating. An explicit user choice always wins over the profile. + if want := modelregistry.ReasoningEffort(profile.ReasoningEffort); want != "" && m.reasoningEffort == "" { + if reasoningEffortAllowed(m.availableReasoningEfforts(), want) { + m.reasoningEffort = want + m.execProfileAppliedEffort = want + } + } + // Self-correction is presence-only: a profile can arm it but never disarm + // an explicit /selfcorrect choice. + if profile.SelfCorrect && !m.selfCorrectTests { + m.selfCorrectTests = true + m.execProfileArmedSelfCorrect = true + } + m.agentOptions.Profile = profile.Policy(displacedMaxTurns, m.execProfileAppliedEffort != "") + m.execProfileName = profile.Name + return m, m.profileText() +} + +// revertExecProfile restores what the active profile displaced. Each knob is +// restored only when the user has not explicitly touched it since the profile +// was applied (the touched bits, set by /turns, /effort, Ctrl+T, and +// /selfcorrect) AND it still holds the value the profile applied — so manual +// overrides survive the revert even when they coincide with the profile's own +// value (e.g. /turns 30 under fast, or /selfcorrect off-then-on under +// thorough). +func (m model) revertExecProfile() model { + if m.execProfileName == "" { + return m + } + if !m.execProfileTurnsTouched && m.execProfileAppliedMaxTurns > 0 && m.agentOptions.MaxTurns == m.execProfileAppliedMaxTurns { + m.agentOptions.MaxTurns = m.execProfileDisplacedMaxTurns + if m.execProfileDisplacedMaxTurns > 0 { + config.SetMaxTurnsEnv(m.execProfileDisplacedMaxTurns) + } else { + // The profile's apply exported its budget to ZERO_MAX_TURNS (for + // sub-agents), and SetMaxTurnsEnv ignores zero — so restoring a + // zero displaced budget must clear the env explicitly or spawned + // children would keep the removed profile's budget. + _ = os.Unsetenv(config.MaxTurnsEnv) + } + } + if !m.execProfileEffortTouched && m.execProfileAppliedEffort != "" && m.reasoningEffort == m.execProfileAppliedEffort { + m.reasoningEffort = "" + } + if !m.execProfileSelfCorrectTouched && m.execProfileArmedSelfCorrect && m.selfCorrectTests { + m.selfCorrectTests = false + } + m.agentOptions.Profile = nil + m.execProfileName = "" + m.execProfileDisplacedMaxTurns = 0 + m.execProfileAppliedMaxTurns = 0 + m.execProfileAppliedEffort = "" + m.execProfileArmedSelfCorrect = false + m.execProfileTurnsTouched = false + m.execProfileEffortTouched = false + m.execProfileSelfCorrectTouched = false + return m +} + +func (m model) profileText() string { + name := m.execProfileName + if name == "" { + name = execprofile.Balanced.Name + " (default)" + } + lines := []string{ + "execution profile: " + name, + fmt.Sprintf("max tool-turns per run: %d", m.agentOptions.MaxTurns), + "reasoning effort: " + m.effortDisplay(), + } + if m.agentOptions.Profile != nil && m.agentOptions.Profile.Escalate != nil { + turnTarget := "keeps the pinned turn budget" + if target := m.agentOptions.Profile.Escalate.MaxTurns; target > 0 { + turnTarget = fmt.Sprintf("restores the turn budget to %d", target) + } + lines = append(lines, + "escalation: armed — one-shot on a tool-failure streak, a failing self-correct cycle, or a critical-risk mutation; "+turnTarget, + "note: the uncertain-completion signal is headless-only (the completion gate never runs interactively), so it cannot fire in the TUI") + } + return renderCommandOutput(commandOutput{ + Title: "Profile", + Status: commandStatusOK, + Sections: []commandSection{{ + Title: "State", + Lines: lines, + }}, + Hints: []string{"/profile " + strings.Join(execprofile.Names(), "|") + " switches the next run's loop posture (turn budget, effort, self-correction, escalation); pick the model separately with /model"}, + }) +} + func (m model) turnsText() string { return renderCommandOutput(commandOutput{ Title: "Turns",