diff --git a/README.md b/README.md index a452911..478d641 100644 --- a/README.md +++ b/README.md @@ -546,6 +546,7 @@ Run `cr config show` to inspect the active profile and credential status. ```yaml profiles: default: + fast: false git: host: github.com auth_mode: pat @@ -1111,7 +1112,8 @@ Modes: | `--rerun` | Bypass existing local approval, approval-override, resume, and marker gates and start a new live review while retaining provider-session reuse. Mutually exclusive with `--retry-posts`. | | `--retry-posts` | Retry missing or failed required posts for an existing run without rerunning LLM planning or checking approval overrides. Mutually exclusive with `--rerun` and incompatible with `--session`. | | `--fresh-session` | Start a fresh provider conversation for this invocation without changing local review gates. Incompatible with `--retry-posts`, which does not run LLM planning. | -| `--fast` | Request fast execution for reviewer agents. Incompatible with `--retry-posts`; unsupported runtime/model combinations fail before LLM planning. | +| `--fast` | Enable fast execution for reviewer agents, overriding the profile default. Incompatible with `--retry-posts`. | +| `--no-fast` | Disable fast execution for reviewer agents, overriding the profile default. Mutually exclusive with `--fast`. | Review selection and execution flags: @@ -1144,18 +1146,24 @@ Policy and output flags: | `--no-resolve-threads` | Do not plan thread-resolution actions. Also implied by profile `resolve_threads: never`. | | `--json` | Emit JSON. | -Fast mode currently supports `claude_cli` and `anthropic_api` with -`claude-opus-4-8` or `claude-opus-4-7`; Anthropic has deprecated Opus 4.7 fast -mode and plans to remove it on July 24, 2026. Claude CLI receives a per-session -`fastMode` setting, while Anthropic API requests use its fast-mode beta. Fast -mode has premium pricing and applies only to reviewer agents, not selection, -synthesis, approval-override classification, or `cr respond` thread analysis. +Fast mode defaults off; set `fast: true` on a profile to enable it by default. +`--fast` and `--no-fast` override the profile. Unsupported runtime/model +combinations warn and continue at normal speed. Fast mode supports `claude_cli` +and `anthropic_api` with `claude-opus-4-8` or `claude-opus-4-7`, and `codex_cli` +with `gpt-5.4`, `gpt-5.5`, `gpt-5.6-sol`, `gpt-5.6-terra`, or `gpt-5.6-luna`. +Anthropic has deprecated Opus 4.7 fast mode and plans to remove it on July 24, +2026. Claude CLI receives a per-session `fastMode` setting, Anthropic API +requests use its fast-mode beta, and Codex CLI receives `service_tier="fast"`. +Fast mode has premium pricing and applies only to reviewer agents, not +selection, synthesis, approval-override classification, or `cr respond` thread +analysis. Anthropic API fast mode is a research preview that requires account access; requests without access fail as upstream errors during the run. Subscription fast mode bills usage credits, requires owner enablement on Team and Enterprise, and can silently degrade to standard speed when credits or fast capacity are -unavailable. Reviewer runtime artifacts therefore record both the fast request -and the speed actually reported by the provider, or `unknown` when unavailable. +unavailable. Reviewer runtime artifacts therefore record the fast request, +whether it was ignored as unsupported, and the speed actually reported by the +provider, or `unknown` when unavailable. Local run state and provider session state are independent. By default, each PR/profile/posting-identity tuple gets one durable provider session shared by diff --git a/docs/init-config-surface.md b/docs/init-config-surface.md index db0da09..7b56bc2 100644 --- a/docs/init-config-surface.md +++ b/docs/init-config-surface.md @@ -57,6 +57,7 @@ intentionally unsupported. | config.profiles..reviewer.github_app_installation.mode | Review profile composition chooses how a GitHub App reviewer entity resolves its installation. | Interactive review-profile flow only for now. | Required when `reviewer.entity` uses GitHub App auth. New GitHub App reviewer selections default to `discover_from_repository`. | `discover_from_repository` resolves from PR repository context; `pinned` requires `installation_id`. Must be empty for PAT and Git identity reviewers. | GitHub App reviewer installation routing tests. | | config.profiles..reviewer.github_app_installation.installation_id | Review profile composition stores an optional pinned GitHub App installation id for this profile. | Interactive review-profile flow only for now. | Empty unless installation mode is `pinned`. | Must be a decimal GitHub App installation id when present. This is profile routing data, not secret material. | GitHub App reviewer installation routing tests. | | config.profiles..llm_runtime | Review profile composition selects one configured LLM runtime by name. | Interactive review-profile flow only for now. Future config commands may own scripted profile composition. | New profiles require an explicit runtime choice. Existing runtime reference is pre-populated. | Must reference `config.llm_runtimes`. API-key runtime credentials must differ from Git and selected reviewer entity credentials when the store also matches. | First-class LLM-runtime reference validation tests. | +| config.profiles..fast | Not shown in interactive init; fast mode is a power-user profile default. | Direct config-file management; `cr review --fast` and `--no-fast` override it per invocation. | Omitted defaults to `false`. | Unsupported runtime/model combinations warn and continue at normal speed. | Profile config and review command precedence tests. | | config.profiles..agent_sources[] | Direct trusted-directory editor shows existing profile agent-source paths in one multiline field with explanatory notes about trust and separate repo-local `.codereview/agents` discovery. | Existing `cr init --agent-source`; existing `cr config agent-source add/remove`; #186 docs sequence. | Omitted means no additional profile-specific trusted directories. Existing values are pre-populated line-by-line. | Preserve on skip. Clearing the field removes all profile-specific agent sources. Entered paths are normalized and deduped through the shared config-edit helper. | #177 extracts helper. #183 tests add/remove/reset/preserve. #289 prompt tests cover explanatory copy, multiline entry, and Back. | | config.profiles..hooks[].event | Not shown in interactive init; lifecycle hooks are power-user config. | Direct config-file management. | No hooks by default. | Must be one of the documented review or respond lifecycle events; the benchmark namespace is reserved. | Hook validation and runtime dispatch tests. | | config.profiles..hooks[].argv[] | Not shown in interactive init. | Direct config-file management. | Required for each hook. | Executed directly without a shell; the command must be non-empty and no argument may contain NUL. | Hook validation and argv execution tests. | diff --git a/internal/app/runtime_test.go b/internal/app/runtime_test.go index e6f5541..00e9daa 100644 --- a/internal/app/runtime_test.go +++ b/internal/app/runtime_test.go @@ -949,6 +949,7 @@ func TestAdapterConstructorsCoverLLMRuntimeSpecs(t *testing.T) { wantFastModels := map[config.LLMAdapter][]string{ config.LLMAdapterClaudeCLI: {"claude-opus-4-8", "claude-opus-4-7"}, config.LLMAdapterAnthropicAPI: {"claude-opus-4-8", "claude-opus-4-7"}, + config.LLMAdapterCodexCLI: {"gpt-5.4", "gpt-5.5", "gpt-5.6-sol", "gpt-5.6-terra", "gpt-5.6-luna"}, } for _, spec := range config.LLMRuntimeSpecs() { if _, duplicate := want[spec.Adapter]; duplicate { diff --git a/internal/cmd/reviewcmd/reviewcmd.go b/internal/cmd/reviewcmd/reviewcmd.go index fb72b80..a824b53 100644 --- a/internal/cmd/reviewcmd/reviewcmd.go +++ b/internal/cmd/reviewcmd/reviewcmd.go @@ -43,8 +43,9 @@ including after pushes and across dry-run/live invocations. --session selects a named live-review session instead; --fresh-session starts a fresh provider conversation for this invocation. ---fast requests fast execution for reviewer agents only. The configured -adapter and every possible reviewer model must support it. +--fast requests fast execution for reviewer agents only; --no-fast disables it. +CLI flags override the profile default. Unsupported runtimes or reviewer models +emit a warning and continue at normal speed. If no existing approval is present and a prior codereview marker exists from the posting identity, cr can fast-path approval when the PR author posts an @@ -66,6 +67,7 @@ type commandFlags struct { retryPosts bool freshSession bool fast bool + noFast bool agentsDirs []string failOn string sessionName string @@ -108,6 +110,7 @@ func RegisterWithFactory(rootCmd *cobra.Command, opts *root.Options, factory Run cmd.Flags().BoolVar(&flags.retryPosts, "retry-posts", false, "Retry missing or failed required posts without rerunning review or checking approval overrides") cmd.Flags().BoolVar(&flags.freshSession, "fresh-session", false, "Start a fresh provider conversation without changing local review gates") cmd.Flags().BoolVar(&flags.fast, "fast", false, "Request fast execution for supported reviewer runtimes and models") + cmd.Flags().BoolVar(&flags.noFast, "no-fast", false, "Disable fast execution for reviewer agents") cmd.Flags().StringArrayVar(&flags.agentsDirs, "agents-dir", nil, "Additional trusted agents directory") cmd.Flags().StringVar(&flags.failOn, "fail-on", "", "Exit 1 when a finding at or above severity exists") cmd.Flags().StringVar(&flags.sessionName, "session", "", "Override the PR's default LLM session with a named live-review session") @@ -129,6 +132,9 @@ func RegisterWithFactory(rootCmd *cobra.Command, opts *root.Options, factory Run } func runReview(ctx context.Context, cmd *cobra.Command, opts *root.Options, factory RuntimeFactory, flags commandFlags, prArg string) error { + if cmd.Flags().Changed("fast") && cmd.Flags().Changed("no-fast") { + return exitcode.Usage(fmt.Errorf("--fast and --no-fast are mutually exclusive")) + } if flags.noPost { flags.dryRun = true } @@ -207,9 +213,6 @@ func runReview(ctx context.Context, cmd *cobra.Command, opts *root.Options, fact if flags.freshSession && flags.retryPosts { return exitcode.Usage(fmt.Errorf("--fresh-session cannot be used with --retry-posts")) } - if flags.fast && flags.retryPosts { - return exitcode.Usage(fmt.Errorf("--fast cannot be used with --retry-posts")) - } if flags.maxAgents < 0 { return exitcode.Usage(fmt.Errorf("--max-agents must be non-negative")) } @@ -266,6 +269,15 @@ func runReview(ctx context.Context, cmd *cobra.Command, opts *root.Options, fact return exitcode.Usage(profileSpan.End(err)) } _ = profileSpan.End(nil) + reviewerFast := profile.Fast + if cmd.Flags().Changed("fast") { + reviewerFast = flags.fast + } else if cmd.Flags().Changed("no-fast") { + reviewerFast = !flags.noFast + } + if reviewerFast && flags.retryPosts { + return exitcode.Usage(fmt.Errorf("fast mode cannot be used with --retry-posts")) + } runtimeReq := app.OpenRequest{ Config: cfg, @@ -316,7 +328,7 @@ func runReview(ctx context.Context, cmd *cobra.Command, opts *root.Options, fact ReviewerModelOverride: reviewerModel, ReviewerModelTierOverride: reviewerModelTier, ReviewerEffortOverride: reviewerEffort, - ReviewerFast: flags.fast, + ReviewerFast: reviewerFast, ReviewBaseSHA: reviewBaseSHA, ReviewHeadSHA: reviewHeadSHA, Rerun: flags.rerun, diff --git a/internal/cmd/reviewcmd/reviewcmd_test.go b/internal/cmd/reviewcmd/reviewcmd_test.go index 71396d6..bf7cb0c 100644 --- a/internal/cmd/reviewcmd/reviewcmd_test.go +++ b/internal/cmd/reviewcmd/reviewcmd_test.go @@ -926,6 +926,69 @@ func TestReviewFastPropagatesToPipelineRequest(t *testing.T) { } } +func TestReviewFastPreferencePrecedence(t *testing.T) { + for _, tt := range []struct { + name string + profileFast bool + flags []string + want bool + }{ + {name: "default off"}, + {name: "profile default", profileFast: true, want: true}, + {name: "flag enables", flags: []string{"--fast"}, want: true}, + {name: "explicit fast false overrides profile", profileFast: true, flags: []string{"--fast=false"}}, + {name: "no-fast overrides profile", profileFast: true, flags: []string{"--no-fast"}}, + {name: "explicit no-fast false enables", flags: []string{"--no-fast=false"}, want: true}, + } { + t.Run(tt.name, func(t *testing.T) { + cfg := testConfig() + profile := cfg.Profiles["home"] + profile.Fast = tt.profileFast + cfg.Profiles["home"] = profile + runner := &fakeRunner{result: testPipelineResult(false)} + cmd, _ := newTestCommand(t, cfg, fakeFactory(runner)) + args := []string{"review", "https://github.com/open-cli-collective/codereview-cli/pull/29", "--dry-run"} + args = append(args, tt.flags...) + if err := root.Execute(cmd, args); err != nil { + t.Fatalf("Execute: %v", err) + } + if len(runner.requests) != 1 || runner.requests[0].ReviewerFast != tt.want { + t.Fatalf("requests = %#v, want ReviewerFast=%t", runner.requests, tt.want) + } + }) + } +} + +func TestReviewRetryPostsUsesEffectiveFastPreference(t *testing.T) { + cfg := testConfig() + profile := cfg.Profiles["home"] + profile.Fast = true + cfg.Profiles["home"] = profile + + t.Run("profile fast rejects before runtime construction", func(t *testing.T) { + factoryCalled := false + cmd, _ := newTestCommand(t, cfg, func(context.Context, app.OpenRequest) (app.Runtime, error) { + factoryCalled = true + return app.Runtime{}, nil + }) + err := root.Execute(cmd, []string{"review", "https://github.com/open-cli-collective/codereview-cli/pull/29", "--retry-posts"}) + if exitcode.FromError(err) != exitcode.UsageError || factoryCalled { + t.Fatalf("Execute error = %v, factory called = %t, want usage before runtime", err, factoryCalled) + } + }) + + t.Run("no-fast permits retry", func(t *testing.T) { + runner := &fakeRunner{liveResult: testLiveResult(false)} + cmd, _ := newTestCommand(t, cfg, fakeFactory(runner)) + if err := root.Execute(cmd, []string{"review", "https://github.com/open-cli-collective/codereview-cli/pull/29", "--retry-posts", "--no-fast"}); err != nil { + t.Fatalf("Execute: %v", err) + } + if len(runner.liveRequests) != 1 || runner.liveRequests[0].ReviewerFast { + t.Fatalf("live requests = %#v, want standard-speed retry", runner.liveRequests) + } + }) +} + func TestReviewLiveSessionPassesNamedSession(t *testing.T) { runner := &fakeRunner{liveResult: testLiveResult(false)} cmd, _ := newTestCommand(t, testConfig(), fakeFactory(runner)) @@ -973,6 +1036,7 @@ func TestReviewRejectsInvalidInputs(t *testing.T) { {name: "session retry posts", args: []string{"review", "https://github.com/open-cli-collective/codereview-cli/pull/29", "--retry-posts", "--session", "daily"}}, {name: "fresh session retry posts", args: []string{"review", "https://github.com/open-cli-collective/codereview-cli/pull/29", "--retry-posts", "--fresh-session"}}, {name: "fast retry posts", args: []string{"review", "https://github.com/open-cli-collective/codereview-cli/pull/29", "--retry-posts", "--fast"}}, + {name: "fast flag conflict", args: []string{"review", "https://github.com/open-cli-collective/codereview-cli/pull/29", "--dry-run", "--fast", "--no-fast"}}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/internal/config/config.go b/internal/config/config.go index 419e866..40b8afd 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -146,6 +146,7 @@ type Profile struct { Git GitConfig `yaml:"-" json:"-"` Reviewer ProfileReviewer `yaml:"reviewer" json:"reviewer"` LLMRuntime string `yaml:"llm_runtime" json:"llm_runtime"` + Fast bool `yaml:"fast,omitempty" json:"fast,omitempty"` AgentSources []string `yaml:"agent_sources,omitempty" json:"agent_sources,omitempty"` ReviewPolicy ReviewPolicy `yaml:"review_policy,omitempty" json:"review_policy"` Hooks []Hook `yaml:"hooks,omitempty" json:"hooks,omitempty"` @@ -163,6 +164,7 @@ type profileYAML struct { Git GitConfig `yaml:"git,omitempty" json:"git,omitempty"` Reviewer ProfileReviewer `yaml:"reviewer" json:"reviewer"` LLMRuntime string `yaml:"llm_runtime" json:"llm_runtime"` + Fast bool `yaml:"fast,omitempty" json:"fast,omitempty"` AgentSources []string `yaml:"agent_sources,omitempty" json:"agent_sources,omitempty"` ReviewPolicy ReviewPolicy `yaml:"review_policy,omitempty" json:"review_policy"` Hooks []Hook `yaml:"hooks,omitempty" json:"hooks,omitempty"` @@ -210,6 +212,7 @@ func (p *Profile) UnmarshalYAML(value *yaml.Node) error { Git: raw.Git, Reviewer: raw.Reviewer, LLMRuntime: raw.LLMRuntime, + Fast: raw.Fast, AgentSources: raw.AgentSources, ReviewPolicy: raw.ReviewPolicy, Hooks: raw.Hooks, @@ -224,6 +227,7 @@ func (p Profile) MarshalYAML() (any, error) { RepositoryAccess: p.RepositoryAccess, Reviewer: p.Reviewer, LLMRuntime: p.LLMRuntime, + Fast: p.Fast, AgentSources: p.AgentSources, ReviewPolicy: p.ReviewPolicy, Hooks: p.Hooks, @@ -561,6 +565,7 @@ var llmRuntimeSpecs = []LLMRuntimeSpec{ string(ModelTierMedium): "gpt-5.4", string(ModelTierLarge): "gpt-5.5", }, + FastModeModels: []string{"gpt-5.4", "gpt-5.5", "gpt-5.6-sol", "gpt-5.6-terra", "gpt-5.6-luna"}, }, { Provider: LLMProviderOpenAI, diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 15dcf1a..c319733 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -217,6 +217,7 @@ func TestLoadAcceptsCodexCLISubscriptionProfile(t *testing.T) { adapter: codex_cli profiles: codex: + fast: true git: host: github.com auth_mode: pat @@ -232,6 +233,20 @@ profiles: t.Fatalf("Load: %v", err) } profile := cfg.Profiles["codex"] + if !profile.Fast { + t.Fatal("Fast = false, want true") + } + savedPath := filepath.Join(t.TempDir(), "saved.yml") + if err := Save(savedPath, cfg); err != nil { + t.Fatalf("Save: %v", err) + } + saved, err := Load(savedPath) + if err != nil { + t.Fatalf("reload saved config: %v", err) + } + if !saved.Profiles["codex"].Fast { + t.Fatal("saved Fast = false, want true") + } if profile.LLM.Provider != LLMProviderOpenAI || profile.LLM.Adapter != LLMAdapterCodexCLI { t.Fatalf("LLM = %#v, want openai/codex_cli", profile.LLM) } diff --git a/internal/llmadapters/subprocess.go b/internal/llmadapters/subprocess.go index 5065712..4d9d24f 100644 --- a/internal/llmadapters/subprocess.go +++ b/internal/llmadapters/subprocess.go @@ -659,6 +659,9 @@ func (a *SubprocessAdapter) buildArgsForSession(req Request, scratch string, res if req.Effort != "" { args = append(args, "-c", "model_reasoning_effort="+req.Effort) } + if req.Fast { + args = append(args, "-c", `service_tier="fast"`) + } if resumeSessionID != "" { args = subprocessCodexResumeArgs() if req.Model != "" { @@ -667,6 +670,9 @@ func (a *SubprocessAdapter) buildArgsForSession(req Request, scratch string, res if req.Effort != "" { args = append(args, "-c", "model_reasoning_effort="+req.Effort) } + if req.Fast { + args = append(args, "-c", `service_tier="fast"`) + } args = append(args, resumeSessionID) } return append(args, "--", req.Prompt), nil diff --git a/internal/llmadapters/subprocess_test.go b/internal/llmadapters/subprocess_test.go index 491ed07..ec28a62 100644 --- a/internal/llmadapters/subprocess_test.go +++ b/internal/llmadapters/subprocess_test.go @@ -13,6 +13,8 @@ import ( "strings" "testing" "time" + + "github.com/open-cli-collective/codereview-cli/internal/llm" ) type trackedAfterFuncContext struct { @@ -764,6 +766,9 @@ func TestSubprocessCodexSafetyModes(t *testing.T) { } } assertFlagValue(t, record.AdapterArgs, "--sandbox", "read-only") + if got := strings.Join(flagValues(record.AdapterArgs, "-c"), ","); got != "model_reasoning_effort=high" { + t.Fatalf("Codex config overrides = %q, want standard-speed reasoning only", got) + } if cd := flagValue(record.AdapterArgs, "--cd"); !samePath(t, cd, record.Cwd) { t.Fatalf("flagValue(--cd) = %q, helper cwd = %q, want same path", cd, record.Cwd) } @@ -828,6 +833,9 @@ func TestSubprocessCodexSafetyModes(t *testing.T) { } assertFlagValue(t, record.AdapterArgs, "--model", "gpt-5.5") assertFlagValue(t, record.AdapterArgs, "-c", "model_reasoning_effort=high") + if got := strings.Join(flagValues(record.AdapterArgs, "-c"), ","); got != "model_reasoning_effort=high" { + t.Fatalf("Codex resume config overrides = %q, want standard-speed reasoning only", got) + } promptIndex := len(argsBeforePrompt(record.AdapterArgs)) if promptIndex+2 != len(record.AdapterArgs) || record.AdapterArgs[promptIndex] != "--" || record.AdapterArgs[promptIndex+1] != "resume prompt" { t.Fatalf("args = %#v, want -- separated resumed prompt", record.AdapterArgs) @@ -837,6 +845,36 @@ func TestSubprocessCodexSafetyModes(t *testing.T) { } }) + for _, tt := range []struct { + name string + sessionID string + }{ + {name: "fresh"}, + {name: "resume", sessionID: "prior-session"}, + } { + t.Run("fast "+tt.name+" uses service tier", func(t *testing.T) { + recordPath := filepath.Join(t.TempDir(), "records.jsonl") + adapter := newCodexHelperAdapter("success", recordPath, 5*time.Second) + req := Request{Model: "gpt-5.5", Effort: "high", Prompt: "prompt", Fast: true} + var stream llm.Stream + var err error + if tt.sessionID == "" { + stream, err = adapter.Start(context.Background(), req) + } else { + stream, err = adapter.Resume(context.Background(), tt.sessionID, req) + } + if err != nil { + t.Fatalf("start: %v", err) + } + if _, err := stream.Wait(context.Background()); err != nil { + t.Fatalf("Wait: %v", err) + } + if got := strings.Join(flagValues(readHelperRecord(t, recordPath).AdapterArgs, "-c"), ","); got != `model_reasoning_effort=high,service_tier="fast"` { + t.Fatalf("Codex config overrides = %q, want fast service tier", got) + } + }) + } + t.Run("resume reviewer workspace is rejected", func(t *testing.T) { tempDir := t.TempDir() scratchRoot := filepath.Join(tempDir, "workbench-scratch") @@ -1469,6 +1507,7 @@ func newCodexHelperAdapter(mode string, recordPath string, timeout time.Duration Env: append(helperEnv(mode, recordPath), extraEnv...), Timeout: timeout, AllowBestEffortNoTools: true, + FastModeModels: []string{"gpt-5.5"}, }) } diff --git a/internal/pipeline/artifacts.go b/internal/pipeline/artifacts.go index 32f78b1..31df959 100644 --- a/internal/pipeline/artifacts.go +++ b/internal/pipeline/artifacts.go @@ -125,17 +125,17 @@ func agentSourcesArtifactFromCatalog(catalog agents.Catalog, reviewerRuntime map return artifact } -func reviewerRuntimeArtifact(req Request, catalog agents.Catalog, selection llm.Selection, fastDelivered string) map[string]reviewerRuntimeResolution { - if req.ReviewerFast && fastDelivered != "fast" && fastDelivered != "standard" { +func reviewerRuntimeArtifact(req Request, catalog agents.Catalog, selection llm.Selection, fastDelivered string, fastRequested, fastIgnored bool) map[string]reviewerRuntimeResolution { + if fastRequested && fastDelivered != "fast" && fastDelivered != "standard" { fastDelivered = "unknown" } if strings.TrimSpace(req.ReviewerModelOverride) != "" { - if !req.ReviewerFast { + if !fastRequested { return nil } out := make(map[string]reviewerRuntimeResolution, len(selection.SelectedAgents)) for _, selected := range selection.SelectedAgents { - out[selected.AgentID] = reviewerRuntimeResolution{Mode: "override", ResolvedModel: strings.TrimSpace(req.ReviewerModelOverride), Fast: true, FastDelivered: fastDelivered} + out[selected.AgentID] = reviewerRuntimeResolution{Mode: "override", ResolvedModel: strings.TrimSpace(req.ReviewerModelOverride), Fast: true, FastIgnored: fastIgnored, FastDelivered: fastDelivered} } return out } @@ -156,8 +156,9 @@ func reviewerRuntimeArtifact(req Request, catalog agents.Catalog, selection llm. if err != nil { continue } - resolution.Fast = req.ReviewerFast - if req.ReviewerFast { + resolution.Fast = fastRequested + resolution.FastIgnored = fastIgnored + if fastRequested { resolution.FastDelivered = fastDelivered } out[selected.AgentID] = resolution diff --git a/internal/pipeline/pipeline.go b/internal/pipeline/pipeline.go index 69d9760..d123b4c 100644 --- a/internal/pipeline/pipeline.go +++ b/internal/pipeline/pipeline.go @@ -355,7 +355,6 @@ type selectionSetupRequest struct { Profile config.Profile PostingIdentity gitprovider.Identity AgentDirs []string - ReviewRequest Request ReviewBaseSHA string ReviewHeadSHA string NoResolveThreads bool @@ -386,6 +385,8 @@ type preparedSelectionContext struct { currentHeadSHA string reviewBaseSHA string reviewHeadSHA string + fastRequested bool + fastIgnored bool } type selectionPhaseRequest struct { @@ -599,7 +600,6 @@ func execute(ctx context.Context, opts Options, req Request, mode executionMode) Profile: req.Profile, PostingIdentity: req.PostingIdentity, AgentDirs: req.AgentDirs, - ReviewRequest: req, ReviewBaseSHA: req.ReviewBaseSHA, ReviewHeadSHA: req.ReviewHeadSHA, NoResolveThreads: req.NoResolveThreads, @@ -618,6 +618,14 @@ func execute(ctx context.Context, opts Options, req Request, mode executionMode) if err != nil { return Result{}, err } + prepared.fastRequested = req.ReviewerFast + effectiveFast, warning, err := resolveReviewerFastMode(req, prepared.catalog) + if err != nil { + return Result{}, err + } + prepared.fastIgnored = req.ReviewerFast && !effectiveFast + req.ReviewerFast = effectiveFast + opts.emitWarning(warning) result := prepared.reviewResult() run := mode.run @@ -776,7 +784,7 @@ func executeLLMPhases(ctx context.Context, opts Options, req Request, mode execu } result.Findings = findings result.ReviewerFailures = reviewerFailures - result.reviewerFastDelivered = reviewerFastDelivery(req.ReviewerFast, reviewerSessions) + result.reviewerFastDelivered = reviewerFastDelivery(prepared.fastRequested, reviewerSessions) reviewerCoverage := buildReviewerCoverage(selection.SelectedAgents, reviewerResults, reviewerFailures, prepared.changedFiles) result.ReviewerCoverage = reviewerCoverage result.Sessions = appendSessionsIfPresent(result.Sessions, reviewerLedgerSessions...) @@ -876,7 +884,7 @@ func persistExecutionResult(ctx context.Context, opts Options, req Request, run return err } result.PlannedActions = plannedActions - return writeArtifacts(prepared.artifacts, prepared.rawDiff, prepared.parsed.Patches, result.Catalog, result.Selection, result.Findings, result.Plan.RollupMarkdown, reviewerRuntimeArtifact(req, prepared.catalog, result.Selection, result.reviewerFastDelivered)) + return writeArtifacts(prepared.artifacts, prepared.rawDiff, prepared.parsed.Patches, result.Catalog, result.Selection, result.Findings, result.Plan.RollupMarkdown, reviewerRuntimeArtifact(req, prepared.catalog, result.Selection, result.reviewerFastDelivered, prepared.fastRequested, prepared.fastIgnored)) } func findIncompleteDryRun(ctx context.Context, store Store, req Request, pr gitprovider.PR) (ledger.Run, bool, error) { @@ -1070,10 +1078,6 @@ func prepareSelectionContext(ctx context.Context, opts Options, req selectionSet return preparedSelectionContext{}, err } opts.emitReviewerCatalog(catalog, patchPaths(parsed.Patches)) - if err := validateReviewerFastMode(req.ReviewRequest, catalog); err != nil { - return preparedSelectionContext{}, err - } - out := preparedSelectionContext{ pr: pr, reviewPR: reviewPR, @@ -2562,28 +2566,26 @@ func resolveReviewerRuntimeConfig(req Request, agent agents.Agent) (llmRuntimeCo return applyStageRuntimeOverrides(req.ReviewerModelOverride, req.ReviewerEffortOverride, resolved.ResolvedModel, agent.Effort), nil } -func validateReviewerFastMode(req Request, catalog agents.Catalog) error { +func resolveReviewerFastMode(req Request, catalog agents.Catalog) (bool, string, error) { if !req.ReviewerFast { - return nil + return false, "", nil } spec, ok := config.FindLLMRuntimeSpec(req.Profile.LLM.Provider, req.Profile.LLM.Auth, req.Profile.LLM.Adapter) runtimeName := fmt.Sprintf("%s/%s/%s", req.Profile.LLM.Provider, req.Profile.LLM.Auth, req.Profile.LLM.Adapter) - if !ok { - return fmt.Errorf("pipeline: --fast is unsupported for runtime %s: runtime is not registered", runtimeName) - } - if len(spec.FastModeModels) == 0 { - return fmt.Errorf("pipeline: --fast is unsupported for runtime %s: adapter has no fast-mode mechanism", runtimeName) + warning := "" + if !ok || len(spec.FastModeModels) == 0 { + warning = fmt.Sprintf("warning: fast mode is unsupported for %s; continuing at normal speed", runtimeName) } for _, agent := range catalog.Agents { runtimeConfig, err := resolveReviewerRuntimeConfig(req, agent) if err != nil { - return err + return false, "", err } - if !spec.SupportsFastMode(runtimeConfig.model) { - return fmt.Errorf("pipeline: --fast is unsupported for runtime %s: reviewer %q resolves to model %q; supported models: %s", runtimeName, agent.ID, runtimeConfig.model, strings.Join(spec.FastModeModels, ", ")) + if warning == "" && (!ok || !spec.SupportsFastMode(runtimeConfig.model)) { + warning = fmt.Sprintf("warning: fast mode is unsupported for %s model %s; continuing at normal speed", runtimeName, runtimeConfig.model) } } - return nil + return warning == "", warning, nil } func resolveAgentModel(profile config.Profile, baselineOverride string, agent agents.Agent) (reviewerRuntimeResolution, error) { diff --git a/internal/pipeline/pipeline_test.go b/internal/pipeline/pipeline_test.go index 7383751..9886ceb 100644 --- a/internal/pipeline/pipeline_test.go +++ b/internal/pipeline/pipeline_test.go @@ -2549,7 +2549,7 @@ func TestDryRunReviewerFloorsResolveIndependentlyPerAgent(t *testing.T) { {AgentID: "harness:reviewer", Files: []string{"main.go"}}, {AgentID: "harness:senior", Files: []string{"main.go"}}, }, - }, "") + }, "", false, false) if got == nil { t.Fatal("reviewerRuntimeArtifact = nil, want selected reviewer runtime metadata") } @@ -2655,6 +2655,7 @@ func TestDryRunFastAppliesOnlyToReviewerAndRecordsArtifact(t *testing.T) { Mode: "override", ResolvedModel: "claude-opus-4-8", Fast: true, + FastIgnored: false, FastDelivered: "standard", }) } @@ -2683,52 +2684,158 @@ func TestReviewerFastDeliveryDegradesConservatively(t *testing.T) { } } -func TestDryRunFastRejectsUnsupportedRuntimeBeforeLLM(t *testing.T) { - tests := []struct { - name string - llm config.LLMConfig - want string - }{ - { - name: "adapter", - llm: config.LLMConfig{Provider: config.LLMProviderOpenAI, Auth: config.LLMAuthSubscription, Adapter: config.LLMAdapterCodexCLI}, - want: "pipeline: --fast is unsupported for runtime openai/subscription/codex_cli: adapter has no fast-mode mechanism", - }, - { - name: "model", - llm: config.LLMConfig{Provider: config.LLMProviderAnthropic, Auth: config.LLMAuthSubscription, Adapter: config.LLMAdapterClaudeCLI}, - want: `pipeline: --fast is unsupported for runtime anthropic/subscription/claude_cli: reviewer "harness:reviewer" resolves to model "claude-sonnet-4-6"; supported models: claude-opus-4-8, claude-opus-4-7`, - }, +func TestDryRunFastFallsBackForUnsupportedModel(t *testing.T) { + ctx := context.Background() + store := openPipelineStore(t) + defer closeStore(t, store) + provider, req := dryRunHarness(t) + req.ReviewerFast = true + adapter := &llm.FakeAdapter{NameValue: "fake-llm"} + adapter.Queue(fakeLLMResult("selection-session", selectionJSON("harness:reviewer", "main.go"), 10, 2)) + adapter.Queue(fakeLLMResult("reviewer-session", findingsJSON("harness:reviewer", "main.go", "major", 2, "Fix this"), 20, 4)) + adapter.Queue(fakeLLMResult("rollup-session", rollupJSON("comment", []string{"finding-1"}), 30, 6)) + var warnings bytes.Buffer + + result, err := dryRunForTest(ctx, Options{ + Provider: provider, + Adapter: adapter, + Store: store, + Layout: statepaths.NewLayout(t.TempDir(), t.TempDir()), + Warnings: &warnings, + Now: fixedNow, + NewRunID: func() string { return "run-fast-unsupported" }, + NewSessionRowID: sequence("session"), + NewFindingID: findingSequence("finding"), + NewActionID: actionSequence(), + MaxConcurrency: 1, + }, req) + if err != nil { + t.Fatalf("DryRun: %v", err) } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - ctx := context.Background() - store := openPipelineStore(t) - defer closeStore(t, store) - provider, req := dryRunHarness(t) - req.Profile.LLM = tt.llm - req.ReviewerFast = true - adapter := &llm.FakeAdapter{NameValue: "fake-llm"} + wantWarning := "warning: fast mode is unsupported for anthropic/subscription/claude_cli model claude-sonnet-4-6; continuing at normal speed\n" + if warnings.String() != wantWarning { + t.Fatalf("warnings = %q, want %q", warnings.String(), wantWarning) + } + requests := adapter.Requests() + if len(requests) != 3 || requests[0].Fast || requests[1].Fast || requests[2].Fast { + t.Fatalf("requests = %#v, want normal-speed fallback", requests) + } + assertReviewerRuntimeArtifact(t, result.Artifacts.AgentSourcesJSON, "harness:reviewer", reviewerRuntimeResolution{ + Mode: "tier_floor", + FloorTier: "medium", + BaselineTier: "small", + EffectiveTier: "medium", + ResolvedModel: "claude-sonnet-4-6", + ModelMapSource: config.ModelMapSourceBuiltIn, + Fast: true, + FastIgnored: true, + FastDelivered: "unknown", + }) + meta, ok, err := llmlifecycle.ReadMetadata(lifecyclePaths(result.Artifacts), reviewerTaskID("harness:reviewer")) + if err != nil || !ok { + t.Fatalf("reviewer metadata = %#v ok %t err %v", meta, ok, err) + } + agent := result.Catalog.Agents[0] + selected := result.Selection.SelectedAgents[0] + prompt, promptDeps, err := buildReviewerPrompt(result.Artifacts, result.PR, selected, agent, []string{"main.go"}) + if err != nil { + t.Fatalf("buildReviewerPrompt: %v", err) + } + deps := append([]string{orchestratorSelectionStage}, promptDeps...) + wantFingerprint := llmlifecycle.Fingerprint(adapter.Name(), reviewerTaskID(agent.ID), "reviewer", requests[1].Model, requests[1].Effort, prompt, deps) + if meta.InputFingerprint != wantFingerprint { + t.Fatalf("reviewer fingerprint = %q, want standard-speed %q", meta.InputFingerprint, wantFingerprint) + } +} - _, err := dryRunForTest(ctx, Options{ - Provider: provider, - Adapter: adapter, - Store: store, - Layout: statepaths.NewLayout(t.TempDir(), t.TempDir()), - Now: fixedNow, - NewRunID: func() string { return "run-fast-unsupported" }, - NewSessionRowID: sequence("session"), - NewFindingID: findingSequence("finding"), - NewActionID: actionSequence(), - MaxConcurrency: 1, - }, req) - if err == nil || !strings.Contains(err.Error(), tt.want) { - t.Fatalf("DryRun error = %v, want %q", err, tt.want) - } - if len(adapter.Requests()) != 0 { - t.Fatalf("LLM requests = %#v, want none", adapter.Requests()) - } - }) +func TestDryRunFastFallsBackForUnsupportedRuntime(t *testing.T) { + ctx := context.Background() + store := openPipelineStore(t) + defer closeStore(t, store) + provider, req := dryRunHarness(t) + req.Profile.LLM = config.LLMConfig{ + Provider: config.LLMProviderPi, + Auth: config.LLMAuthSubscription, + Adapter: config.LLMAdapterPiRPC, + ModelMap: config.ModelMap{"medium": "pi-model"}, + } + req.ReviewerModelOverride = "pi-model" + req.ReviewerFast = true + adapter := &llm.FakeAdapter{NameValue: "fake-llm"} + adapter.Queue(fakeLLMResult("selection-session", selectionJSON("harness:reviewer", "main.go"), 10, 2)) + adapter.Queue(fakeLLMResult("reviewer-session", findingsJSON("harness:reviewer", "main.go", "major", 2, "Fix this"), 20, 4)) + adapter.Queue(fakeLLMResult("rollup-session", rollupJSON("comment", []string{"finding-1"}), 30, 6)) + var warnings bytes.Buffer + + result, err := dryRunForTest(ctx, Options{ + Provider: provider, + Adapter: adapter, + Store: store, + Layout: statepaths.NewLayout(t.TempDir(), t.TempDir()), + Warnings: &warnings, + Now: fixedNow, + NewRunID: func() string { return "run-fast-unsupported-runtime" }, + NewSessionRowID: sequence("session"), + NewFindingID: findingSequence("finding"), + NewActionID: actionSequence(), + MaxConcurrency: 1, + }, req) + if err != nil { + t.Fatalf("DryRun: %v", err) + } + wantWarning := "warning: fast mode is unsupported for pi/subscription/pi_rpc; continuing at normal speed\n" + if warnings.String() != wantWarning { + t.Fatalf("warnings = %q, want %q", warnings.String(), wantWarning) + } + requests := adapter.Requests() + if len(requests) != 3 || requests[0].Fast || requests[1].Fast || requests[2].Fast { + t.Fatalf("requests = %#v, want normal-speed fallback", requests) + } + assertReviewerRuntimeArtifact(t, result.Artifacts.AgentSourcesJSON, "harness:reviewer", reviewerRuntimeResolution{ + Mode: "override", + ResolvedModel: "pi-model", + Fast: true, + FastIgnored: true, + FastDelivered: "unknown", + }) +} + +func TestDryRunFastPreflightResolvesEveryReviewerBeforeLLM(t *testing.T) { + ctx := context.Background() + store := openPipelineStore(t) + defer closeStore(t, store) + provider, req := dryRunHarness(t) + writeAgentWithModelTier(t, req.Profile.AgentSources[0], "harness", "unmapped", "small") + req.ReviewerFast = true + adapter := &llm.FakeAdapter{NameValue: "fake-llm"} + + _, err := dryRunForTest(ctx, Options{ + Provider: provider, + Adapter: adapter, + Store: store, + Layout: statepaths.NewLayout(t.TempDir(), t.TempDir()), + Now: fixedNow, + NewRunID: func() string { return "run-fast-preflight" }, + NewSessionRowID: sequence("session"), + NewFindingID: findingSequence("finding"), + NewActionID: actionSequence(), + MaxConcurrency: 1, + }, req) + if err == nil || !strings.Contains(err.Error(), `model_tier "small" is not mapped`) { + t.Fatalf("DryRun error = %v, want later reviewer model resolution failure", err) + } + if len(adapter.Requests()) != 0 { + t.Fatalf("LLM requests = %#v, want preflight failure before selection", adapter.Requests()) + } +} + +func TestResolveReviewerFastModeRejectsUnsupportedRuntimeWithoutAgents(t *testing.T) { + _, req := dryRunHarness(t) + req.ReviewerFast = true + req.Profile.LLM = config.LLMConfig{Provider: config.LLMProviderPi, Auth: config.LLMAuthSubscription, Adapter: config.LLMAdapterPiRPC} + effective, warning, err := resolveReviewerFastMode(req, agents.Catalog{}) + if err != nil || effective || warning != "warning: fast mode is unsupported for pi/subscription/pi_rpc; continuing at normal speed" { + t.Fatalf("resolveReviewerFastMode = (%t, %q, %v), want unsupported runtime fallback", effective, warning, err) } } diff --git a/internal/pipeline/prompts.go b/internal/pipeline/prompts.go index 437154a..7bd9ee4 100644 --- a/internal/pipeline/prompts.go +++ b/internal/pipeline/prompts.go @@ -577,6 +577,7 @@ type reviewerRuntimeResolution struct { ResolvedModel string `json:"resolved_model"` ModelMapSource config.ModelMapSource `json:"model_map_source,omitempty"` Fast bool `json:"fast,omitempty"` + FastIgnored bool `json:"fast_ignored,omitempty"` FastDelivered string `json:"fast_delivered,omitempty"` }