From 7e1acba29ec7e799bfb83fc089f3841f347d7025 Mon Sep 17 00:00:00 2001 From: Jacob Kim Date: Mon, 27 Jul 2026 16:36:06 +0900 Subject: [PATCH] feat(ui): select model effort in compare and review dialogs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Compare and Review could pick a model but never a reasoning effort, so both always ran at the provider's configured tier. - add src/lib/providers/model-effort.ts as the single source for supported effort values, settings-derived defaults, cross-model clamping, and provider runtime-override mapping - give ModelSelector an opt-in effort axis so the trigger reads "model · effort" and the search dialog gains an effort row - replace the Compare provider/model dropdown pair with the searchable ModelSelector, per candidate and for the independent judge - carry candidate effort into task runtime overrides and judge effort into the judge runtime options, dropping values a model rejects - expand review focuses to seven with per-item descriptions sourced from the same module that builds the prompt instructions - drop the hardcoded bg-surface on the Compare textareas so they match the Review dialog in every theme Co-Authored-By: Claude Opus 5 --- .../local-change-review-dialog.tsx | 96 +++++--- src/components/ai-elements/model-selector.tsx | 108 ++++++++- src/components/compare/CompareRunPanel.tsx | 2 + .../compare/CompareRunPrepareDialog.tsx | 214 +++++++++++------- src/components/session/ChatInput.tsx | 6 + src/lib/compare-runs.ts | 51 ++++- src/lib/local-change-review.ts | 55 ++++- src/lib/providers/model-effort.ts | 170 ++++++++++++++ src/store/app.store.ts | 7 +- src/store/compare-run-judge.ts | 11 + src/store/compare-run-start.ts | 19 +- tests/compare-run-store.test.ts | 107 +++++++++ tests/compare-runs.test.ts | 27 +++ tests/local-change-review.test.ts | 22 +- tests/model-effort.test.ts | 113 +++++++++ tests/model-selector-effort.test.tsx | 61 +++++ 16 files changed, 936 insertions(+), 133 deletions(-) create mode 100644 src/lib/providers/model-effort.ts create mode 100644 tests/model-effort.test.ts create mode 100644 tests/model-selector-effort.test.tsx diff --git a/src/components/ai-elements/local-change-review-dialog.tsx b/src/components/ai-elements/local-change-review-dialog.tsx index 561e13f6..ebf00da5 100644 --- a/src/components/ai-elements/local-change-review-dialog.tsx +++ b/src/components/ai-elements/local-change-review-dialog.tsx @@ -25,12 +25,19 @@ import { isSourceControlUntracked, type SourceControlStatusItem, } from "@/lib/source-control-status"; -import type { - LocalChangeReviewFocus, - LocalChangeReviewScope, +import { + LOCAL_CHANGE_REVIEW_FOCUS_OPTIONS, + type LocalChangeReviewFocus, + type LocalChangeReviewScope, } from "@/lib/local-change-review"; +import { + clampModelEffort, + resolveModelEffortFromSettings, + type ModelEffort, +} from "@/lib/providers/model-effort"; import { getProviderLabel } from "@/lib/providers/model-catalog"; import { cn } from "@/lib/utils"; +import { useAppStore } from "@/store/app.store"; import { ModelIcon } from "./model-icon"; import { ModelSelector, type ModelSelectorOption } from "./model-selector"; @@ -39,17 +46,6 @@ const DEFAULT_REVIEW_FOCUSES: readonly LocalChangeReviewFocus[] = [ "tests", ]; -const REVIEW_FOCUS_OPTIONS: ReadonlyArray<{ - value: LocalChangeReviewFocus; - label: string; -}> = [ - { value: "correctness", label: "Correctness" }, - { value: "tests", label: "Test gaps" }, - { value: "security", label: "Security" }, - { value: "performance", label: "Performance" }, - { value: "architecture", label: "Architecture" }, -]; - const REVIEW_SCOPE_OPTIONS: ReadonlyArray<{ value: LocalChangeReviewScope; label: string; @@ -77,6 +73,7 @@ type ReviewChangeStatus = export interface LocalChangeReviewRequest { reviewer: ModelSelectorOption; + effort: ModelEffort; scope: LocalChangeReviewScope; focuses: readonly LocalChangeReviewFocus[]; instructions?: string; @@ -121,6 +118,7 @@ export function LocalChangeReviewDialog(args: LocalChangeReviewDialogProps) { const idPrefix = useId(); const [open, setOpen] = useState(false); const [reviewerKey, setReviewerKey] = useState(); + const [selectedEffort, setSelectedEffort] = useState(); const [scope, setScope] = useState("working-tree"); const [focuses, setFocuses] = useState( DEFAULT_REVIEW_FOCUSES, @@ -132,10 +130,25 @@ export function LocalChangeReviewDialog(args: LocalChangeReviewDialogProps) { }); const statusRequestIdRef = useRef(0); + const settings = useAppStore((state) => state.settings); const preferredReviewer = getPreferredReviewer(args); const reviewer = args.reviewerOptions.find((option) => option.key === reviewerKey) ?? preferredReviewer; + // The reviewer's own model preference is the default; an explicit pick is + // kept across provider switches and clamped to what the new model accepts. + const effort = reviewer + ? clampModelEffort({ + providerId: reviewer.providerId, + model: reviewer.model, + effort: selectedEffort, + fallback: resolveModelEffortFromSettings({ + settings, + providerId: reviewer.providerId, + model: reviewer.model, + }), + }) + : undefined; const providerIds = useMemo( () => [...new Set(args.reviewerOptions.map((option) => option.providerId))], [args.reviewerOptions], @@ -227,13 +240,14 @@ export function LocalChangeReviewDialog(args: LocalChangeReviewDialogProps) { } async function handleSubmit() { - if (!reviewer || isSubmitting) { + if (!reviewer || !effort || isSubmitting) { return; } setIsSubmitting(true); try { const submitted = await args.onSubmit({ reviewer, + effort, scope, focuses, instructions: instructions.trim() || undefined, @@ -247,7 +261,7 @@ export function LocalChangeReviewDialog(args: LocalChangeReviewDialogProps) { } } - if (!reviewer) { + if (!reviewer || !effort) { return null; } @@ -387,8 +401,8 @@ export function LocalChangeReviewDialog(args: LocalChangeReviewDialogProps) { Review by

- Choose any available provider and model. This does not change - the task's active provider. + Choose any available provider, model, and reasoning effort. This + does not change the task's active provider.

@@ -425,7 +439,11 @@ export function LocalChangeReviewDialog(args: LocalChangeReviewDialogProps) { setReviewerKey(selection.key)} + effort={effort} + onSelect={({ selection, effort: nextEffort }) => { + setReviewerKey(selection.key); + setSelectedEffort(nextEffort); + }} className="w-full" triggerClassName="h-11 w-full max-w-none border-border/80 bg-background px-3" menuClassName="sm:max-w-lg" @@ -438,24 +456,46 @@ export function LocalChangeReviewDialog(args: LocalChangeReviewDialogProps) { Focus

- Select the signals that should receive extra scrutiny. + Each selected focus adds an explicit instruction to the review + prompt. Unselected areas are still read for context.

-
- {REVIEW_FOCUS_OPTIONS.map((option) => { +
+ {LOCAL_CHANGE_REVIEW_FOCUS_OPTIONS.map((option) => { const selected = focuses.includes(option.value); return ( - + + + + {option.label} + + + {option.description} + + + ); })}
diff --git a/src/components/ai-elements/model-selector.tsx b/src/components/ai-elements/model-selector.tsx index 5173c91f..b1b2f976 100644 --- a/src/components/ai-elements/model-selector.tsx +++ b/src/components/ai-elements/model-selector.tsx @@ -1,5 +1,5 @@ import { ChevronDown, Sparkles } from "lucide-react"; -import { useEffect, useMemo, useRef, useState } from "react"; +import { useEffect, useId, useMemo, useRef, useState } from "react"; import { Command, CommandEmpty, @@ -17,6 +17,12 @@ import { DialogTitle, DialogTrigger, } from "@/components/ui/dialog"; +import { + clampModelEffort, + getModelEffortLabel, + listModelEffortOptions, + type ModelEffort, +} from "@/lib/providers/model-effort"; import { getProviderLabel, listProviderIds, @@ -46,7 +52,16 @@ interface ModelSelectorProps { triggerAriaLabel?: string; menuClassName?: string; openToken?: string | number; - onSelect: (args: { selection: ModelSelectorOption }) => void; + /** + * Opt-in effort axis. When provided, the trigger shows `model · effort` and + * the dialog gains an effort row; picking a model re-clamps the effort to a + * value that model accepts before it reaches `onSelect`. + */ + effort?: ModelEffort; + onSelect: (args: { + selection: ModelSelectorOption; + effort?: ModelEffort; + }) => void; } export function ModelSelector(args: ModelSelectorProps) { @@ -60,9 +75,11 @@ export function ModelSelector(args: ModelSelectorProps) { triggerAriaLabel, menuClassName, openToken, + effort, onSelect, } = args; const [open, setOpen] = useState(false); + const effortGroupId = useId(); // Seed the handled token with the value present at mount time. `openToken` is // a one-shot "open now" trigger owned by the parent, but the parent keeps the // latched value across the selector's mount/unmount cycles (e.g. when an @@ -95,16 +112,51 @@ export function ModelSelector(args: ModelSelectorProps) { .filter(([, providerOptions]) => providerOptions.length > 0); }, [options, recommendedOptionKeys]); + const effortEnabled = effort !== undefined && !value.isAuto; + const effortOptions = useMemo( + () => + effortEnabled + ? listModelEffortOptions({ + providerId: value.providerId, + model: value.model, + }) + : [], + [effortEnabled, value.model, value.providerId], + ); + const effortLabel = effortEnabled + ? getModelEffortLabel({ + providerId: value.providerId, + model: value.model, + effort, + }) + : undefined; + + const selectOption = (option: ModelSelectorOption) => { + onSelect({ + selection: option, + ...(effort !== undefined + ? { + effort: option.isAuto + ? effort + : clampModelEffort({ + providerId: option.providerId, + model: option.model, + effort, + fallback: effort, + }), + } + : {}), + }); + setOpen(false); + }; + const renderOption = (option: ModelSelectorOption) => ( { - onSelect({ selection: option }); - setOpen(false); - }} + onSelect={() => selectOption(option)} className="gap-3 rounded-lg px-3 py-2.5" > {option.isAuto ? ( @@ -176,7 +228,10 @@ export function ModelSelector(args: ModelSelectorProps) { className="size-3.5" /> )} - {value.label} + + {value.label} + {effortLabel ? ` · ${effortLabel}` : ""} + @@ -230,6 +285,45 @@ export function ModelSelector(args: ModelSelectorProps) { ))} + {effortEnabled && effortOptions.length > 0 ? ( +
+ + Effort + +
+ {effortOptions.map((option) => { + const selected = option.value === effort; + return ( + + ); + })} +
+
+ ) : null} ); diff --git a/src/components/compare/CompareRunPanel.tsx b/src/components/compare/CompareRunPanel.tsx index ecab051b..4cc15e88 100644 --- a/src/components/compare/CompareRunPanel.tsx +++ b/src/components/compare/CompareRunPanel.tsx @@ -613,6 +613,7 @@ export function CompareRunPanel(props: CompareRunPanelProps) { {displayedJudgeModel ? ` · ${toHumanModelName({ model: displayedJudgeModel })}` : ""} + {judge.effort ? ` · ${judge.effort}` : ""} {" · Fresh context · Read only"} {judge.status === "completed" && judgeProvenance ? ` · Rubric v${judgeProvenance.rubricVersion} · Attempt ${judgeProvenance.attempt}` @@ -727,6 +728,7 @@ export function CompareRunPanel(props: CompareRunPanelProps) { {variant.model ? ` / ${toHumanModelName({ model: variant.model })}` : ""} + {variant.effort ? ` · ${variant.effort}` : ""}

{candidateScore ? ( diff --git a/src/components/compare/CompareRunPrepareDialog.tsx b/src/components/compare/CompareRunPrepareDialog.tsx index 379c1214..9589a495 100644 --- a/src/components/compare/CompareRunPrepareDialog.tsx +++ b/src/components/compare/CompareRunPrepareDialog.tsx @@ -5,12 +5,12 @@ import { ShieldCheck, SplitSquareHorizontal, } from "lucide-react"; -import { useState } from "react"; -import { useShallow } from "zustand/react/shallow"; +import { useMemo, useState } from "react"; import { - pickDefaultModelForProvider, - ProviderModelPicker, -} from "@/components/session/ProviderModelPicker"; + buildModelSelectorOptions, + buildModelSelectorValue, + ModelSelector, +} from "@/components/ai-elements/model-selector"; import { Button, Textarea } from "@/components/ui"; import { Dialog, @@ -27,9 +27,13 @@ import { type CompareRunJudgeConfig, type CompareRunVariantConfig, } from "@/lib/compare-runs"; -import type { ProviderId } from "@/lib/providers/provider.types"; +import { listProviderIds } from "@/lib/providers/model-catalog"; +import { resolveModelEffortFromSettings } from "@/lib/providers/model-effort"; +import { useCodexModelCatalog } from "@/lib/providers/use-codex-model-catalog"; import { useAppStore } from "@/store/app.store"; +const COMPARE_PROVIDER_IDS = listProviderIds(); + export interface CompareRunPreparation { seedPrompt: string; variants: CompareRunVariantConfig[]; @@ -50,38 +54,61 @@ export function CompareRunPrepareDialog(props: CompareRunPrepareDialogProps) { const [criteriaDraft, setCriteriaDraft] = useState( DEFAULT_COMPARE_REVIEW_CRITERIA.join("\n"), ); - const [ - activeWorkspaceId, - workspaces, - workspaceBranchById, - modelClaude, - modelCodex, - ] = useAppStore( - useShallow((state) => [ - state.activeWorkspaceId, - state.workspaces, - state.workspaceBranchById, - state.settings.modelClaude, - state.settings.modelCodex, - ]), + const activeWorkspaceId = useAppStore((state) => state.activeWorkspaceId); + const workspaces = useAppStore((state) => state.workspaces); + const workspaceBranchById = useAppStore((state) => state.workspaceBranchById); + const providerAvailability = useAppStore( + (state) => state.providerAvailability, ); + const settings = useAppStore((state) => state.settings); + const { modelClaude, modelCodex } = settings; const workspace = workspaces.find((entry) => entry.id === activeWorkspaceId); - const [candidates, setCandidates] = useState([ - { - provider: "claude-code", - model: modelClaude, - label: "Candidate A", - }, - { - provider: "codex", - model: modelCodex, - label: "Candidate B", - }, - ]); - const [judge, setJudge] = useState({ + const codexModelCatalog = useCodexModelCatalog({ + enabled: props.open, + codexBinaryPath: settings.codexBinaryPath, + }); + const modelOptions = useMemo( + () => + buildModelSelectorOptions({ + providerIds: COMPARE_PROVIDER_IDS, + availabilityByProvider: providerAvailability, + modelsByProvider: { codex: codexModelCatalog.models }, + }), + [codexModelCatalog.models, providerAvailability], + ); + const [candidates, setCandidates] = useState( + () => [ + { + provider: "claude-code", + model: modelClaude, + effort: resolveModelEffortFromSettings({ + settings, + providerId: "claude-code", + model: modelClaude, + }), + label: "Candidate A", + }, + { + provider: "codex", + model: modelCodex, + effort: resolveModelEffortFromSettings({ + settings, + providerId: "codex", + model: modelCodex, + }), + label: "Candidate B", + }, + ], + ); + const [judge, setJudge] = useState(() => ({ provider: "codex", model: modelCodex, - }); + effort: resolveModelEffortFromSettings({ + settings, + providerId: "codex", + model: modelCodex, + }), + })); const baseBranch = workspaceBranchById[activeWorkspaceId] || workspace?.name || @@ -106,17 +133,14 @@ export function CompareRunPrepareDialog(props: CompareRunPrepareDialogProps) { ); } - function selectCandidateProvider(index: number, provider: ProviderId) { - updateCandidate(index, { - provider, - model: pickDefaultModelForProvider(provider), - }); - } - - function selectJudgeProvider(provider: ProviderId) { - setJudge({ - provider, - model: pickDefaultModelForProvider(provider), + function buildSelectorValue(config: { + provider: CompareRunVariantConfig["provider"]; + model?: string; + }) { + return buildModelSelectorValue({ + providerId: config.provider, + model: config.model ?? "", + available: providerAvailability[config.provider], }); } @@ -166,7 +190,7 @@ export function CompareRunPrepareDialog(props: CompareRunPrepareDialogProps) { aria-label="Compare shared brief" value={preparedPrompt} onChange={(event) => setPreparedPrompt(event.target.value)} - className="min-h-28 resize-y bg-surface px-3 py-2.5 leading-6" + className="min-h-28 resize-y px-3 py-2.5 leading-6" /> @@ -181,7 +205,7 @@ export function CompareRunPrepareDialog(props: CompareRunPrepareDialogProps) {

Both start from the same branch with independent files and - provider sessions. + provider sessions. Pick a model and reasoning effort for each.

@@ -190,36 +214,46 @@ export function CompareRunPrepareDialog(props: CompareRunPrepareDialogProps) {
- {candidates.map((candidate, index) => ( -
- - {String(index + 1).padStart(2, "0")} - -
-

- {candidate.label} -

-

- Isolated worktree -

+ {candidates.map((candidate, index) => { + const candidateName = + candidate.label ?? `Candidate ${index + 1}`; + const candidateModel = buildSelectorValue(candidate); + return ( +
+ + {String(index + 1).padStart(2, "0")} + +
+

+ {candidate.label} +

+

+ Isolated worktree +

+
+ + updateCandidate(index, { + provider: selection.providerId, + model: selection.model, + effort, + }) + } + className="w-full" + triggerAriaLabel={`${candidateName} model and effort: ${candidateModel.label}${candidate.effort ? ` · ${candidate.effort}` : ""}`} + triggerClassName="h-9 w-full max-w-none border-input bg-background px-3" + menuClassName="sm:max-w-lg" + />
- - selectCandidateProvider(index, provider) - } - onModelChange={(model) => updateCandidate(index, { model })} - disabled={props.submitting} - providerSelectClassName="h-9 w-[9.5rem] shrink-0" - modelSelectClassName="h-9" - /> -
- ))} + ); + })}
@@ -242,17 +276,22 @@ export function CompareRunPrepareDialog(props: CompareRunPrepareDialogProps) {

- - setJudge((current) => ({ ...current, model })) - } + + setJudge({ + provider: selection.providerId, + model: selection.model, + effort, + }) + } + className="w-full" + triggerAriaLabel={`Independent judge model and effort: ${buildSelectorValue(judge).label}${judge.effort ? ` · ${judge.effort}` : ""}`} + triggerClassName="h-9 w-full max-w-none border-input bg-background px-3" + menuClassName="sm:max-w-lg" /> @@ -277,7 +316,7 @@ export function CompareRunPrepareDialog(props: CompareRunPrepareDialogProps) { aria-label="Compare review criteria" value={criteriaDraft} onChange={(event) => setCriteriaDraft(event.target.value)} - className="min-h-24 resize-y bg-surface px-3 py-2.5 leading-6" + className="min-h-24 resize-y px-3 py-2.5 leading-6" /> @@ -314,6 +353,7 @@ export function CompareRunPrepareDialog(props: CompareRunPrepareDialogProps) { judge: { provider: judge.provider, model: judge.model?.trim(), + effort: judge.effort, }, reviewCriteria, }) diff --git a/src/components/session/ChatInput.tsx b/src/components/session/ChatInput.tsx index d5adac43..48f72868 100644 --- a/src/components/session/ChatInput.tsx +++ b/src/components/session/ChatInput.tsx @@ -59,6 +59,7 @@ import { type ProviderModePresetId, } from "@/lib/providers/provider-mode-presets"; import { applyModelRuntimePreference } from "@/lib/providers/model-runtime-preferences"; +import { buildModelEffortRuntimeOverrides } from "@/lib/providers/model-effort"; import type { ClaudeSettingSource } from "@/lib/providers/provider.types"; import { getCachedProviderCommandCatalog, @@ -2140,6 +2141,11 @@ function BaseChatInput() { runtimeOverrides: { autoRouting: false, model: review.reviewer.model, + ...buildModelEffortRuntimeOverrides({ + providerId: review.reviewer.providerId, + model: review.reviewer.model, + effort: review.effort, + }), }, preservePromptDraft: true, }); diff --git a/src/lib/compare-runs.ts b/src/lib/compare-runs.ts index 11426894..5fe37a36 100644 --- a/src/lib/compare-runs.ts +++ b/src/lib/compare-runs.ts @@ -1,3 +1,9 @@ +import { + clampModelEffort, + isClaudeModelEffort, + isCodexModelEffort, + type ModelEffort, +} from "@/lib/providers/model-effort"; import type { NormalizedProviderEvent, ProviderId, @@ -6,12 +12,16 @@ import type { export interface CompareRunVariantConfig { provider: ProviderId; model?: string; + /** Reasoning effort for this candidate. Missing on pre-effort runs. */ + effort?: ModelEffort; label?: string; } export interface CompareRunJudgeConfig { provider: ProviderId; model?: string; + /** Reasoning effort for the judge turn. Missing on pre-effort runs. */ + effort?: ModelEffort; } export type CompareRunJudgeStatus = @@ -183,6 +193,34 @@ export function buildDefaultCompareVariants(args: { ]; } +/** + * Repairs an effort the target model cannot run. A Codex tier steps down to + * the nearest supported one; a value the provider has no ladder for at all + * (e.g. Codex "ultra" on a Claude candidate) is dropped so the run falls back + * to that provider's configured effort instead of a guessed tier. + */ +function normalizeCompareEffort(args: { + provider: ProviderId; + model: string | undefined; + effort: ModelEffort | undefined; +}): ModelEffort | undefined { + if (!args.effort || !args.model) { + return undefined; + } + if (args.provider === "claude-code") { + return isClaudeModelEffort(args.effort) ? args.effort : undefined; + } + if (!isCodexModelEffort(args.effort)) { + return undefined; + } + return clampModelEffort({ + providerId: args.provider, + model: args.model, + effort: args.effort, + fallback: args.effort, + }); +} + export function normalizeCompareVariants( variants: CompareRunVariantConfig[] | undefined, ): CompareRunVariantConfig[] { @@ -190,10 +228,16 @@ export function normalizeCompareVariants( if (variant.provider !== "claude-code" && variant.provider !== "codex") { return []; } + const model = variant.model?.trim() || undefined; return [ { provider: variant.provider, - model: variant.model?.trim() || undefined, + model, + effort: normalizeCompareEffort({ + provider: variant.provider, + model, + effort: variant.effort, + }), label: variant.label?.trim() || undefined, }, ]; @@ -209,9 +253,11 @@ export function normalizeCompareJudgeConfig( judge?.provider === "claude-code" || judge?.provider === "codex" ? judge.provider : "codex"; + const model = judge?.model?.trim() || undefined; return { provider, - model: judge?.model?.trim() || undefined, + model, + effort: normalizeCompareEffort({ provider, model, effort: judge?.effort }), }; } @@ -759,6 +805,7 @@ export function buildInitialCompareRun(args: { id: buildCompareRunVariantId({ compareRunId: args.id, index }), provider: variant.provider, model: variant.model, + effort: variant.effort, label: variant.label, status: "pending", })), diff --git a/src/lib/local-change-review.ts b/src/lib/local-change-review.ts index 879195d2..5504beca 100644 --- a/src/lib/local-change-review.ts +++ b/src/lib/local-change-review.ts @@ -5,7 +5,9 @@ export type LocalChangeReviewFocus = | "tests" | "security" | "performance" - | "architecture"; + | "architecture" + | "ui-accessibility" + | "error-handling"; const REVIEW_FOCUS_INSTRUCTIONS: Record = { correctness: @@ -18,8 +20,59 @@ const REVIEW_FOCUS_INSTRUCTIONS: Record = { "Performance: meaningful hot-path, memory, I/O, or rendering regressions.", architecture: "Architecture: contract drift, misplaced responsibilities, and changes that violate repository guidance.", + "ui-accessibility": + "UI and accessibility: hardcoded colors instead of theme tokens, contrast or dark-mode breakage, keyboard and screen-reader gaps, and layout regressions.", + "error-handling": + "Error handling: unhandled failure paths, swallowed errors, missing cancellation or timeout handling, and states that cannot recover from a partial failure.", }; +/** + * Single source of truth for the focus chips. The description tells the user + * what the reviewer is actually instructed to do, so it stays a plain-language + * restatement of `REVIEW_FOCUS_INSTRUCTIONS`. + */ +export const LOCAL_CHANGE_REVIEW_FOCUS_OPTIONS: ReadonlyArray<{ + value: LocalChangeReviewFocus; + label: string; + description: string; +}> = [ + { + value: "correctness", + label: "Correctness", + description: "Logic errors, regressions, races, and data loss.", + }, + { + value: "tests", + label: "Test gaps", + description: "Missing coverage that could hide a real regression.", + }, + { + value: "security", + label: "Security", + description: "Unsafe input, secret exposure, permission bypasses.", + }, + { + value: "performance", + label: "Performance", + description: "Hot-path, memory, I/O, and rendering regressions.", + }, + { + value: "architecture", + label: "Architecture", + description: "Contract drift and repository-guideline violations.", + }, + { + value: "ui-accessibility", + label: "UI & accessibility", + description: "Theme tokens, keyboard/screen-reader, layout breakage.", + }, + { + value: "error-handling", + label: "Error handling", + description: "Failure paths, cancellation, timeouts, recovery.", + }, +]; + function buildScopeInstructions(scope: LocalChangeReviewScope) { if (scope === "branch") { return [ diff --git a/src/lib/providers/model-effort.ts b/src/lib/providers/model-effort.ts new file mode 100644 index 00000000..676e03c1 --- /dev/null +++ b/src/lib/providers/model-effort.ts @@ -0,0 +1,170 @@ +import { + clampCodexEffortToModel, + resolveDefaultClaudeEffortForModel, + resolveDefaultCodexEffortForModel, +} from "@/lib/providers/model-catalog"; +import { + applyModelRuntimePreference, + type ModelRuntimePreferenceSettings, +} from "@/lib/providers/model-runtime-preferences"; +import type { + ProviderId, + ProviderRuntimeOptions, +} from "@/lib/providers/provider.types"; +import { + CLAUDE_EFFORT_OPTIONS, + CODEX_EFFORT_OPTIONS, + listCodexEffortOptionsForModel, +} from "@/lib/providers/runtime-option-contract"; + +/** + * Provider-agnostic model effort helpers. + * + * Surfaces that launch a one-off run (local change review, compare candidates, + * compare judge, workspace kickoff) all need the same three things: the effort + * values a provider/model pair accepts, a sane default derived from settings, + * and the runtime-override shape the provider runtime expects. Keeping that in + * one module stops each surface from re-deriving provider branches. + */ + +export type ClaudeModelEffort = NonNullable< + ProviderRuntimeOptions["claudeEffort"] +>; +export type CodexModelEffort = NonNullable< + ProviderRuntimeOptions["codexReasoningEffort"] +>; +export type ModelEffort = ClaudeModelEffort | CodexModelEffort; + +export interface ModelEffortOption { + value: ModelEffort; + label: string; +} + +const CLAUDE_EFFORT_VALUES = new Set( + CLAUDE_EFFORT_OPTIONS.map((option) => option.value), +); +// "minimal" is a valid runtime value that the selector list intentionally +// hides, so the guard has to accept it even though it is not offered. +const CODEX_EFFORT_VALUES = new Set([ + "minimal", + ...CODEX_EFFORT_OPTIONS.map((option) => option.value), +]); + +export function isClaudeModelEffort( + value: string | undefined, +): value is ClaudeModelEffort { + return value != null && CLAUDE_EFFORT_VALUES.has(value); +} + +export function isCodexModelEffort( + value: string | undefined, +): value is CodexModelEffort { + return value != null && CODEX_EFFORT_VALUES.has(value); +} + +export function isModelEffort(value: string | undefined): value is ModelEffort { + return isClaudeModelEffort(value) || isCodexModelEffort(value); +} + +/** The effort the provider itself recommends for this model. */ +export function resolveDefaultModelEffort(args: { + providerId: ProviderId; + model: string; +}): ModelEffort { + return args.providerId === "claude-code" + ? resolveDefaultClaudeEffortForModel({ model: args.model }) + : resolveDefaultCodexEffortForModel({ model: args.model }); +} + +/** Effort values the given provider/model pair actually accepts. */ +export function listModelEffortOptions(args: { + providerId: ProviderId; + model: string; +}): readonly ModelEffortOption[] { + return args.providerId === "claude-code" + ? CLAUDE_EFFORT_OPTIONS + : listCodexEffortOptionsForModel({ model: args.model }); +} + +/** The effort a task would run at today, honoring per-model preferences. */ +export function resolveModelEffortFromSettings< + TSettings extends ModelRuntimePreferenceSettings, +>(args: { + settings: TSettings; + providerId: ProviderId; + model: string; +}): ModelEffort { + const runtimeSettings = applyModelRuntimePreference(args); + return args.providerId === "claude-code" + ? runtimeSettings.claudeEffort + : runtimeSettings.codexReasoningEffort; +} + +/** + * Keeps a carried-over effort valid after a model or provider switch: prefer + * the requested value, step down to the nearest supported Codex tier, then the + * caller's default, and finally the model's own recommended effort. The last + * step never escalates to the most expensive tier just because a + * cross-provider value (e.g. Codex "ultra" on Claude) could not be honored. + */ +export function clampModelEffort(args: { + providerId: ProviderId; + model: string; + effort: ModelEffort | undefined; + fallback: ModelEffort; +}): ModelEffort { + const options = listModelEffortOptions({ + providerId: args.providerId, + model: args.model, + }); + const supports = (value: ModelEffort | undefined) => + value != null && options.some((option) => option.value === value); + + if (supports(args.effort)) { + return args.effort as ModelEffort; + } + if (args.providerId === "codex" && isCodexModelEffort(args.effort)) { + return clampCodexEffortToModel({ model: args.model, effort: args.effort }); + } + if (supports(args.fallback)) { + return args.fallback; + } + const providerDefault = resolveDefaultModelEffort(args); + return supports(providerDefault) + ? providerDefault + : (options[0]?.value ?? providerDefault); +} + +/** Runtime override patch for the provider that owns this effort value. */ +export function buildModelEffortRuntimeOverrides(args: { + providerId: ProviderId; + model: string; + effort: ModelEffort | undefined; +}): Pick { + if (!args.effort) { + return {}; + } + if (args.providerId === "claude-code") { + return isClaudeModelEffort(args.effort) + ? { claudeEffort: args.effort } + : {}; + } + return isCodexModelEffort(args.effort) + ? { + codexReasoningEffort: clampCodexEffortToModel({ + model: args.model, + effort: args.effort, + }), + } + : {}; +} + +export function getModelEffortLabel(args: { + providerId: ProviderId; + model: string; + effort: ModelEffort | undefined; +}): string | undefined { + return listModelEffortOptions(args).find( + (option) => option.value === args.effort, + )?.label; +} diff --git a/src/store/app.store.ts b/src/store/app.store.ts index 340607be..888f7d39 100644 --- a/src/store/app.store.ts +++ b/src/store/app.store.ts @@ -6922,11 +6922,8 @@ export const useAppStore = create()( : null; }, setTaskProvider: (input) => get().setTaskProvider(input), - setTaskModel: ({ taskId, model }) => - get().updatePromptDraft({ - taskId, - patch: { runtimeOverrides: { model } }, - }), + setTaskRuntimeOverrides: ({ taskId, runtimeOverrides }) => + get().updatePromptDraft({ taskId, patch: { runtimeOverrides } }), sendUserMessage: (input) => get().sendUserMessage(input), }); diff --git a/src/store/compare-run-judge.ts b/src/store/compare-run-judge.ts index 698133de..f6c1cfae 100644 --- a/src/store/compare-run-judge.ts +++ b/src/store/compare-run-judge.ts @@ -7,6 +7,10 @@ import { type CompareRun, } from "@/lib/compare-runs"; import { getDefaultModelForProvider } from "@/lib/providers/model-catalog"; +import { + buildModelEffortRuntimeOverrides, + type ModelEffort, +} from "@/lib/providers/model-effort"; import type { ProviderId, ProviderRuntimeOptions, @@ -121,6 +125,7 @@ export function buildCompareJudgePrompt(run: CompareRun) { export function buildCompareJudgeRuntimeOptions(args: { provider: ProviderId; model: string; + effort?: ModelEffort; settings: CompareJudgeRuntimeSettings; }): ProviderRuntimeOptions { const base = buildProviderRuntimeOptions({ @@ -131,6 +136,11 @@ export function buildCompareJudgeRuntimeOptions(args: { return { ...base, model: args.model, + ...buildModelEffortRuntimeOverrides({ + providerId: args.provider, + model: args.model, + effort: args.effort, + }), chatStreamingEnabled: false, responseStylePrompt: undefined, promptPrDescription: undefined, @@ -333,6 +343,7 @@ async function executeCompareJudge(args: { const runtimeOptions = buildCompareJudgeRuntimeOptions({ provider: judge.provider, model, + effort: judge.effort, settings: args.settings, }); diff --git a/src/store/compare-run-start.ts b/src/store/compare-run-start.ts index 7834136a..ff5c6140 100644 --- a/src/store/compare-run-start.ts +++ b/src/store/compare-run-start.ts @@ -3,7 +3,9 @@ import { type CompareRun, type CompareRunVariant, } from "@/lib/compare-runs"; +import { buildModelEffortRuntimeOverrides } from "@/lib/providers/model-effort"; import type { ProviderId } from "@/lib/providers/provider.types"; +import type { PromptDraftRuntimeOverrides } from "@/types/chat"; type VariantStatus = CompareRunVariant["status"]; @@ -39,7 +41,10 @@ export async function launchCompareRunVariants(args: { fallbackWorkspaceName: string, ) => CandidateWorkspaceSnapshot | null; setTaskProvider: (input: { taskId: string; provider: ProviderId }) => void; - setTaskModel: (input: { taskId: string; model: string }) => void; + setTaskRuntimeOverrides: (input: { + taskId: string; + runtimeOverrides: PromptDraftRuntimeOverrides; + }) => void; sendUserMessage: (input: { taskId: string; content: string; @@ -105,7 +110,17 @@ export async function launchCompareRunVariants(args: { }); const model = variant.model?.trim(); if (model) { - args.setTaskModel({ taskId: candidate.taskId, model }); + args.setTaskRuntimeOverrides({ + taskId: candidate.taskId, + runtimeOverrides: { + model, + ...buildModelEffortRuntimeOverrides({ + providerId: variant.provider, + model, + effort: variant.effort, + }), + }, + }); } const launchResult = await args.sendUserMessage({ diff --git a/tests/compare-run-store.test.ts b/tests/compare-run-store.test.ts index fc02991b..8cabba25 100644 --- a/tests/compare-run-store.test.ts +++ b/tests/compare-run-store.test.ts @@ -311,6 +311,113 @@ describe("compare run store actions", () => { ]); }); + test("carries each candidate effort into its task runtime overrides", async () => { + const runtimeOverridePatches: Array<{ + taskId: string; + runtimeOverrides: unknown; + }> = []; + let workspaceIndex = 0; + + useAppStore.setState({ + createWorkspace: async (args) => { + workspaceIndex += 1; + const workspaceId = `workspace-${workspaceIndex}`; + const taskId = `task-${workspaceIndex}`; + useAppStore.setState((state) => ({ + workspaces: [ + ...state.workspaces, + { + id: workspaceId, + name: args.name, + updatedAt: "2026-06-18T00:00:00.000Z", + }, + ], + activeWorkspaceId: workspaceId, + activeTaskId: taskId, + activeSurface: { kind: "task", taskId }, + tasks: [buildTask({ id: taskId, title: `Task ${workspaceIndex}` })], + taskWorkspaceIdById: { + ...state.taskWorkspaceIdById, + [taskId]: workspaceId, + }, + })); + return { ok: true }; + }, + setTaskProvider: () => {}, + updatePromptDraft: ({ taskId, patch }) => { + runtimeOverridePatches.push({ + taskId, + runtimeOverrides: patch.runtimeOverrides, + }); + }, + sendUserMessage: async (args) => ({ + status: "started", + taskId: args.taskId, + workspaceId: "workspace-1", + turnId: "turn-1", + }), + }); + + const result = await useAppStore.getState().startCompareRun({ + seedPrompt: "Compare with explicit effort", + variants: [ + { + provider: "claude-code", + model: "claude-sonnet-5", + effort: "max", + label: "A", + }, + { + provider: "codex", + model: "gpt-5.6-luna", + // Luna rejects "ultra"; the run must step it down, not send it. + effort: "ultra", + label: "B", + }, + ], + judge: { provider: "codex", model: "gpt-5.6-luna", effort: "ultra" }, + }); + + expect(result.ok).toBe(true); + expect(runtimeOverridePatches).toEqual([ + { + taskId: "task-1", + runtimeOverrides: { model: "claude-sonnet-5", claudeEffort: "max" }, + }, + { + taskId: "task-2", + runtimeOverrides: { + model: "gpt-5.6-luna", + codexReasoningEffort: "max", + }, + }, + ]); + expect( + useAppStore.getState().compareRunsById[result.compareRunId ?? ""]?.judge + ?.effort, + ).toBe("max"); + }); + + test("applies the judge effort to its runtime options", () => { + expect( + buildCompareJudgeRuntimeOptions({ + provider: "codex", + model: "gpt-5.6-sol", + effort: "ultra", + settings: useAppStore.getState().settings, + }).codexReasoningEffort, + ).toBe("ultra"); + + expect( + buildCompareJudgeRuntimeOptions({ + provider: "claude-code", + model: "claude-sonnet-5", + effort: "max", + settings: useAppStore.getState().settings, + }).claudeEffort, + ).toBe("max"); + }); + test("fails each throwing candidate without leaving the run pending", async () => { let createAttempts = 0; useAppStore.setState({ diff --git a/tests/compare-runs.test.ts b/tests/compare-runs.test.ts index 35702f9c..05281bf0 100644 --- a/tests/compare-runs.test.ts +++ b/tests/compare-runs.test.ts @@ -7,6 +7,7 @@ import { finalizeCompareRunLaunch, finishCompareVariantForTask, isCompareJudgeReady, + normalizeCompareJudgeConfig, normalizeCompareReviewCriteria, normalizePersistedCompareRuns, normalizeCompareVariants, @@ -64,6 +65,32 @@ describe("compare run helpers", () => { ]); }); + test("keeps a supported effort and repairs one the model rejects", () => { + expect( + normalizeCompareVariants([ + { provider: "codex", model: "gpt-5.6-sol", effort: "ultra" }, + { provider: "codex", model: "gpt-5.6-luna", effort: "ultra" }, + { + provider: "claude-code", + model: "claude-sonnet-5", + effort: "ultra", + }, + ]).map((variant) => variant.effort), + ).toEqual(["ultra", "max", undefined]); + + expect( + normalizeCompareJudgeConfig({ + provider: "codex", + model: "gpt-5.6-luna", + effort: "ultra", + }), + ).toEqual({ + provider: "codex", + model: "gpt-5.6-luna", + effort: "max", + }); + }); + test("derives stable run titles and workspace names", () => { expect( deriveCompareSeedTitle("\n Implement provider compare runs\n"), diff --git a/tests/local-change-review.test.ts b/tests/local-change-review.test.ts index 2ac18160..358e061f 100644 --- a/tests/local-change-review.test.ts +++ b/tests/local-change-review.test.ts @@ -1,5 +1,8 @@ import { describe, expect, test } from "bun:test"; -import { buildLocalChangeReviewPrompt } from "@/lib/local-change-review"; +import { + buildLocalChangeReviewPrompt, + LOCAL_CHANGE_REVIEW_FOCUS_OPTIONS, +} from "@/lib/local-change-review"; describe("local change review prompt", () => { test("defaults the review source to the uncommitted working tree", () => { @@ -31,4 +34,21 @@ describe("local change review prompt", () => { expect(prompt).toContain("Check the renderer to preload contract."); expect(prompt).toContain('say "No findings" explicitly'); }); + + test("every selectable focus is described and reaches the prompt", () => { + for (const option of LOCAL_CHANGE_REVIEW_FOCUS_OPTIONS) { + expect(option.description.length).toBeGreaterThan(0); + } + + const prompt = buildLocalChangeReviewPrompt({ + scope: "working-tree", + focuses: LOCAL_CHANGE_REVIEW_FOCUS_OPTIONS.map((option) => option.value), + }); + + expect(prompt).toContain("UI and accessibility:"); + expect(prompt).toContain("Error handling:"); + expect( + prompt.split("\n").filter((line) => line.startsWith("- ")), + ).toHaveLength(LOCAL_CHANGE_REVIEW_FOCUS_OPTIONS.length); + }); }); diff --git a/tests/model-effort.test.ts b/tests/model-effort.test.ts new file mode 100644 index 00000000..f2bacffe --- /dev/null +++ b/tests/model-effort.test.ts @@ -0,0 +1,113 @@ +import { describe, expect, test } from "bun:test"; +import { + buildModelEffortRuntimeOverrides, + clampModelEffort, + getModelEffortLabel, + isModelEffort, + listModelEffortOptions, +} from "@/lib/providers/model-effort"; + +describe("model effort helpers", () => { + test("lists provider-specific effort values", () => { + expect( + listModelEffortOptions({ + providerId: "claude-code", + model: "claude-sonnet-5", + }).map((option) => option.value), + ).toEqual(["low", "medium", "high", "xhigh", "max"]); + + expect( + listModelEffortOptions({ + providerId: "codex", + model: "gpt-5.6-luna", + }).map((option) => option.value), + ).not.toContain("ultra"); + }); + + test("keeps a supported effort and steps down an unsupported Codex tier", () => { + expect( + clampModelEffort({ + providerId: "codex", + model: "gpt-5.6-sol", + effort: "ultra", + fallback: "medium", + }), + ).toBe("ultra"); + + expect( + clampModelEffort({ + providerId: "codex", + model: "gpt-5.6-luna", + effort: "ultra", + fallback: "medium", + }), + ).toBe("max"); + }); + + test("falls back when the provider cannot run the carried-over effort", () => { + expect( + clampModelEffort({ + providerId: "claude-code", + model: "claude-sonnet-5", + effort: "ultra", + fallback: "high", + }), + ).toBe("high"); + + expect( + clampModelEffort({ + providerId: "claude-code", + model: "claude-sonnet-5", + effort: undefined, + fallback: "max", + }), + ).toBe("max"); + }); + + test("maps effort onto the runtime override the provider reads", () => { + expect( + buildModelEffortRuntimeOverrides({ + providerId: "claude-code", + model: "claude-sonnet-5", + effort: "xhigh", + }), + ).toEqual({ claudeEffort: "xhigh" }); + + expect( + buildModelEffortRuntimeOverrides({ + providerId: "codex", + model: "gpt-5.6-luna", + effort: "ultra", + }), + ).toEqual({ codexReasoningEffort: "max" }); + + expect( + buildModelEffortRuntimeOverrides({ + providerId: "claude-code", + model: "claude-sonnet-5", + effort: "ultra", + }), + ).toEqual({}); + + expect( + buildModelEffortRuntimeOverrides({ + providerId: "codex", + model: "gpt-5.6-sol", + effort: undefined, + }), + ).toEqual({}); + }); + + test("labels efforts and rejects unknown values", () => { + expect( + getModelEffortLabel({ + providerId: "claude-code", + model: "claude-sonnet-5", + effort: "xhigh", + }), + ).toBe("X-High"); + expect(isModelEffort("ultra")).toBe(true); + expect(isModelEffort("turbo")).toBe(false); + expect(isModelEffort(undefined)).toBe(false); + }); +}); diff --git a/tests/model-selector-effort.test.tsx b/tests/model-selector-effort.test.tsx new file mode 100644 index 00000000..775df0b1 --- /dev/null +++ b/tests/model-selector-effort.test.tsx @@ -0,0 +1,61 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { createElement } from "react"; +import { renderToStaticMarkup } from "react-dom/server"; + +const originalWindow = globalThis.window; + +function setWindowContext() { + (globalThis as { window?: unknown }).window = { + api: {}, + location: { href: "https://stave.test/workspace" }, + } as unknown; +} + +describe("ModelSelector effort axis", () => { + test("shows the effort next to the model on the trigger", async () => { + setWindowContext(); + const { ModelSelector, buildModelSelectorValue } = await import( + "@/components/ai-elements/model-selector" + ); + const value = buildModelSelectorValue({ + providerId: "codex", + model: "gpt-5.6-sol", + }); + + const html = renderToStaticMarkup( + createElement(ModelSelector, { + value, + options: [value], + effort: "xhigh", + onSelect: () => {}, + }), + ); + + expect(html).toContain("· X-High"); + }); + + test("omits the effort suffix when the caller opts out", async () => { + setWindowContext(); + const { ModelSelector, buildModelSelectorValue } = await import( + "@/components/ai-elements/model-selector" + ); + const value = buildModelSelectorValue({ + providerId: "codex", + model: "gpt-5.6-sol", + }); + + const html = renderToStaticMarkup( + createElement(ModelSelector, { + value, + options: [value], + onSelect: () => {}, + }), + ); + + expect(html).not.toContain("·"); + }); +}); + +afterEach(() => { + (globalThis as { window?: unknown }).window = originalWindow; +});