From 76672bd20aa7d18b56e1dbfac9c7b83fb1f1fc68 Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Tue, 21 Jul 2026 16:12:00 -0400 Subject: [PATCH 1/5] feat(review): make fast mode best effort Closes #524 --- README.md | 26 ++++-- docs/init-config-surface.md | 1 + internal/app/runtime_test.go | 1 + internal/cmd/reviewcmd/reviewcmd.go | 25 ++++-- internal/cmd/reviewcmd/reviewcmd_test.go | 62 +++++++++++++ internal/config/config.go | 5 ++ internal/config/config_test.go | 4 + internal/llmadapters/subprocess.go | 6 ++ internal/llmadapters/subprocess_test.go | 39 +++++++++ internal/pipeline/artifacts.go | 13 +-- internal/pipeline/pipeline.go | 36 ++++---- internal/pipeline/pipeline_test.go | 107 +++++++++++++---------- internal/pipeline/prompts.go | 1 + 13 files changed, 241 insertions(+), 85 deletions(-) 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..21df75c 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,16 @@ 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 flags.fast { + reviewerFast = true + } + if flags.noFast { + reviewerFast = false + } + if reviewerFast && flags.retryPosts { + return exitcode.Usage(fmt.Errorf("fast mode cannot be used with --retry-posts")) + } runtimeReq := app.OpenRequest{ Config: cfg, @@ -316,7 +329,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..9ccb478 100644 --- a/internal/cmd/reviewcmd/reviewcmd_test.go +++ b/internal/cmd/reviewcmd/reviewcmd_test.go @@ -926,6 +926,67 @@ 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: "no-fast overrides profile", profileFast: true, flags: []string{"--no-fast"}}, + } { + 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 +1034,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..76e0ddb 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,9 @@ profiles: t.Fatalf("Load: %v", err) } profile := cfg.Profiles["codex"] + if !profile.Fast { + t.Fatal("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..180fd59 100644 --- a/internal/pipeline/pipeline.go +++ b/internal/pipeline/pipeline.go @@ -386,6 +386,8 @@ type preparedSelectionContext struct { currentHeadSHA string reviewBaseSHA string reviewHeadSHA string + fastRequested bool + fastIgnored bool } type selectionPhaseRequest struct { @@ -618,6 +620,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 +786,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 +886,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 +1080,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 +2568,22 @@ 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) - } 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 !ok || !spec.SupportsFastMode(runtimeConfig.model) { + return false, fmt.Sprintf("warning: fast mode is unsupported for %s model %s; continuing at normal speed", runtimeName, runtimeConfig.model), nil } } - return nil + return true, "", 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..6277abe 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") } @@ -2683,52 +2683,67 @@ 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`, - }, - } - 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"} +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 - _, 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()) - } - }) + 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) + } + 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) } } 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"` } From 52dae81de7a1b44e6fc3928880724147abf1997f Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Tue, 21 Jul 2026 16:25:35 -0400 Subject: [PATCH 2/5] fix(review): preserve fast preflight errors --- internal/cmd/reviewcmd/reviewcmd.go | 9 ++++----- internal/cmd/reviewcmd/reviewcmd_test.go | 2 ++ internal/config/config_test.go | 11 +++++++++++ internal/pipeline/pipeline.go | 7 ++++--- internal/pipeline/pipeline_test.go | 13 +++++++++++++ 5 files changed, 34 insertions(+), 8 deletions(-) diff --git a/internal/cmd/reviewcmd/reviewcmd.go b/internal/cmd/reviewcmd/reviewcmd.go index 21df75c..a824b53 100644 --- a/internal/cmd/reviewcmd/reviewcmd.go +++ b/internal/cmd/reviewcmd/reviewcmd.go @@ -270,11 +270,10 @@ func runReview(ctx context.Context, cmd *cobra.Command, opts *root.Options, fact } _ = profileSpan.End(nil) reviewerFast := profile.Fast - if flags.fast { - reviewerFast = true - } - if flags.noFast { - reviewerFast = false + 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")) diff --git a/internal/cmd/reviewcmd/reviewcmd_test.go b/internal/cmd/reviewcmd/reviewcmd_test.go index 9ccb478..bf7cb0c 100644 --- a/internal/cmd/reviewcmd/reviewcmd_test.go +++ b/internal/cmd/reviewcmd/reviewcmd_test.go @@ -936,7 +936,9 @@ func TestReviewFastPreferencePrecedence(t *testing.T) { {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() diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 76e0ddb..c319733 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -236,6 +236,17 @@ profiles: 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/pipeline/pipeline.go b/internal/pipeline/pipeline.go index 180fd59..327080d 100644 --- a/internal/pipeline/pipeline.go +++ b/internal/pipeline/pipeline.go @@ -2574,16 +2574,17 @@ func resolveReviewerFastMode(req Request, catalog agents.Catalog) (bool, string, } 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) + warning := "" for _, agent := range catalog.Agents { runtimeConfig, err := resolveReviewerRuntimeConfig(req, agent) if err != nil { return false, "", err } - if !ok || !spec.SupportsFastMode(runtimeConfig.model) { - return false, fmt.Sprintf("warning: fast mode is unsupported for %s model %s; continuing at normal speed", runtimeName, runtimeConfig.model), nil + 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 true, "", 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 6277abe..76e04aa 100644 --- a/internal/pipeline/pipeline_test.go +++ b/internal/pipeline/pipeline_test.go @@ -2747,6 +2747,19 @@ func TestDryRunFastFallsBackForUnsupportedModel(t *testing.T) { } } +func TestResolveReviewerFastModeDoesNotHideLaterModelErrors(t *testing.T) { + _, req := dryRunHarness(t) + req.ReviewerFast = true + catalog := agents.Catalog{Agents: []agents.Agent{ + {ID: "harness:unsupported", ModelTier: "medium"}, + {ID: "harness:invalid", ModelTier: "invalid"}, + }} + _, _, err := resolveReviewerFastMode(req, catalog) + if err == nil || !strings.Contains(err.Error(), `model_tier "invalid" is invalid`) { + t.Fatalf("resolveReviewerFastMode error = %v, want later model resolution failure", err) + } +} + func TestDryRunPrunesConfiguredRetentionBeforeFetch(t *testing.T) { ctx := context.Background() store := openPipelineStore(t) From 8fcccac9ddb435ff77939ff7747fb5147f89e62d Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Tue, 21 Jul 2026 16:29:00 -0400 Subject: [PATCH 3/5] fix(pipeline): handle unsupported empty catalogs --- internal/pipeline/pipeline.go | 5 +++-- internal/pipeline/pipeline_test.go | 10 ++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/internal/pipeline/pipeline.go b/internal/pipeline/pipeline.go index 327080d..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 @@ -601,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, @@ -2575,6 +2573,9 @@ func resolveReviewerFastMode(req Request, catalog agents.Catalog) (bool, string, 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) 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 { diff --git a/internal/pipeline/pipeline_test.go b/internal/pipeline/pipeline_test.go index 76e04aa..8c1b169 100644 --- a/internal/pipeline/pipeline_test.go +++ b/internal/pipeline/pipeline_test.go @@ -2760,6 +2760,16 @@ func TestResolveReviewerFastModeDoesNotHideLaterModelErrors(t *testing.T) { } } +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) + } +} + func TestDryRunPrunesConfiguredRetentionBeforeFetch(t *testing.T) { ctx := context.Background() store := openPipelineStore(t) From 02a93c3c1c175b8ca47f59381efb6f14dfb6f6b5 Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Tue, 21 Jul 2026 16:33:57 -0400 Subject: [PATCH 4/5] test(pipeline): cover fast fallback boundaries --- internal/pipeline/pipeline_test.go | 82 ++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/internal/pipeline/pipeline_test.go b/internal/pipeline/pipeline_test.go index 8c1b169..89a7c9c 100644 --- a/internal/pipeline/pipeline_test.go +++ b/internal/pipeline/pipeline_test.go @@ -2655,6 +2655,7 @@ func TestDryRunFastAppliesOnlyToReviewerAndRecordsArtifact(t *testing.T) { Mode: "override", ResolvedModel: "claude-opus-4-8", Fast: true, + FastIgnored: false, FastDelivered: "standard", }) } @@ -2747,6 +2748,87 @@ func TestDryRunFastFallsBackForUnsupportedModel(t *testing.T) { } } +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 TestResolveReviewerFastModeDoesNotHideLaterModelErrors(t *testing.T) { _, req := dryRunHarness(t) req.ReviewerFast = true From c130f4638513a19ebcd1127cb1fabb8be3270f55 Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Tue, 21 Jul 2026 16:41:00 -0400 Subject: [PATCH 5/5] test(pipeline): remove redundant fast helper coverage --- internal/pipeline/pipeline_test.go | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/internal/pipeline/pipeline_test.go b/internal/pipeline/pipeline_test.go index 89a7c9c..9886ceb 100644 --- a/internal/pipeline/pipeline_test.go +++ b/internal/pipeline/pipeline_test.go @@ -2829,19 +2829,6 @@ func TestDryRunFastPreflightResolvesEveryReviewerBeforeLLM(t *testing.T) { } } -func TestResolveReviewerFastModeDoesNotHideLaterModelErrors(t *testing.T) { - _, req := dryRunHarness(t) - req.ReviewerFast = true - catalog := agents.Catalog{Agents: []agents.Agent{ - {ID: "harness:unsupported", ModelTier: "medium"}, - {ID: "harness:invalid", ModelTier: "invalid"}, - }} - _, _, err := resolveReviewerFastMode(req, catalog) - if err == nil || !strings.Contains(err.Error(), `model_tier "invalid" is invalid`) { - t.Fatalf("resolveReviewerFastMode error = %v, want later model resolution failure", err) - } -} - func TestResolveReviewerFastModeRejectsUnsupportedRuntimeWithoutAgents(t *testing.T) { _, req := dryRunHarness(t) req.ReviewerFast = true