diff --git a/packages/ui/src/components/chat/ModelControls.tsx b/packages/ui/src/components/chat/ModelControls.tsx index d09b0004..880b9475 100644 --- a/packages/ui/src/components/chat/ModelControls.tsx +++ b/packages/ui/src/components/chat/ModelControls.tsx @@ -326,7 +326,13 @@ export const ModelControls: React.FC = ({ const currentModelId = useConfigStore((state) => state.currentModelId); const effectiveCurrentVariant = useConfigStore((state) => state.currentVariant); const currentVariantSelection = useConfigStore((state) => state.currentVariantSelection); - const currentVariant = currentVariantSelection.override ?? undefined; + // What the picker shows is what the next send carries: an explicit choice + // when there is one, "Default" when "Default" was picked, and otherwise the + // inherited effort — showing "Default" while an inherited effort is in + // force is how a switch away from it looks like it did not stick. + const currentVariant = currentVariantSelection.override === null + ? undefined + : currentVariantSelection.override ?? effectiveCurrentVariant; const currentAgentName = useConfigStore((state) => state.currentAgentName); const settingsDefaultVariant = useConfigStore((state) => state.settingsDefaultVariant); const settingsDefaultAgent = useConfigStore((state) => state.settingsDefaultAgent); @@ -728,6 +734,10 @@ export const ModelControls: React.FC = ({ const effectiveAgentName = uiAgentName || currentAgentName; if (currentSessionId && effectiveAgentName) { const savedVariant = getAgentModelVariantForSession(currentSessionId, effectiveAgentName, providerId, modelId); + // An explicit "Default" is a choice: it stops the fallbacks below. + if (savedVariant === null) { + return undefined; + } if (savedVariant && variantOptions.includes(savedVariant)) { return savedVariant; } @@ -777,7 +787,9 @@ export const ModelControls: React.FC = ({ const effectiveAgentName = agentNameOverride ?? resolveLiveAgentName(); if (currentSessionId && effectiveAgentName) { - saveAgentModelVariantForSession(currentSessionId, effectiveAgentName, providerId, modelId, variant); + // `null`, not `undefined`: picking "Default" is a choice to record, + // not the absence of one. + saveAgentModelVariantForSession(currentSessionId, effectiveAgentName, providerId, modelId, variant ?? null); } }, [ addRecentEffort, @@ -1113,11 +1125,10 @@ export const ModelControls: React.FC = ({ return; } + // The chosen effort does not exist on this model: drop the choice and + // inherit, rather than pin an explicit "Default" the user never picked. if (currentVariant && !availableVariants.includes(currentVariant)) { - setCurrentVariantOverride( - null, - resolveInheritedVariantForModel(currentProviderId, currentModelId), - ); + setCurrentVariant(resolveInheritedVariantForModel(currentProviderId, currentModelId)); return; } @@ -1143,7 +1154,8 @@ export const ModelControls: React.FC = ({ const inheritedVariant = resolveInheritedVariantForModel(currentProviderId, currentModelId); if (savedVariant && availableVariants.includes(savedVariant)) { setCurrentVariantOverride(savedVariant, inheritedVariant); - } else if (currentVariantSelection.override === null) { + } else if (savedVariant === null || currentVariantSelection.override === null) { + // "Default" was picked for this session, or is picked right now. setCurrentVariantOverride(null, inheritedVariant); } else { setCurrentVariant(inheritedVariant); diff --git a/packages/ui/src/hooks/useQueuedMessageAutoSend.ts b/packages/ui/src/hooks/useQueuedMessageAutoSend.ts index 87bee797..623aaa8e 100644 --- a/packages/ui/src/hooks/useQueuedMessageAutoSend.ts +++ b/packages/ui/src/hooks/useQueuedMessageAutoSend.ts @@ -148,11 +148,18 @@ const resolveSessionSendConfig = (sessionId: string) => { ?? config.currentModelId ?? selection.lastUsedProvider?.modelID; - const variant = + // A recorded `null` is an explicit "Default": it stops the lookup and sends + // no effort, instead of falling through to the persisted copy. + const savedVariant = selectedAgent && providerID && modelID - ? (selection.getAgentModelVariantForSession(sessionId, selectedAgent, providerID, modelID) - ?? context.getAgentModelVariantForSession(sessionId, selectedAgent, providerID, modelID)) + ? (() => { + const live = selection.getAgentModelVariantForSession(sessionId, selectedAgent, providerID, modelID); + return live !== undefined + ? live + : context.getAgentModelVariantForSession(sessionId, selectedAgent, providerID, modelID); + })() : undefined; + const variant = savedVariant ?? undefined; return { providerID, diff --git a/packages/ui/src/stores/DOCUMENTATION.md b/packages/ui/src/stores/DOCUMENTATION.md index cf3964b0..96152f62 100644 --- a/packages/ui/src/stores/DOCUMENTATION.md +++ b/packages/ui/src/stores/DOCUMENTATION.md @@ -218,11 +218,22 @@ Each of them therefore keeps two things: tracks the **active** project only. Thinking variants keep the effective value in `currentVariant` so existing send -paths capture a stable configuration. The transient `currentVariantSelection` -distinguishes automatic initialization from a picker or shortcut choosing an -explicit override or `Default`; returning to `Default` restores its inherited -effective value. Only explicit overrides are stored in the per-session -selection store. +paths capture a stable configuration. `currentVariantSelection` says where that +value came from: a string is an effort chosen in the picker or by the shortcut, +`null` is an explicit `Default`, and `undefined` is automatic initialization, +which lets the inherited default apply. + +`Default` sends no effort at all. It cannot resolve back to the inherited +default: the settings default would take effect again, and the next assistant +reply echoes that effort back as an explicit choice, so the picker jumps off +`Default` one message after the user chose it. For the same reason the +per-session selection store records an explicit `Default` (as `null`) instead of +clearing the entry — a cleared entry is indistinguishable from never having +chosen, and the settings default wins again on the next agent or session switch. + +Every write of `currentVariant` writes `currentVariantSelection` with it. They +are one selection; updating only the effective value leaves the picker showing +one effort while sends carry another. Every loader and mutation takes an explicit directory; omitting it means the active project, which is what non-Settings callers pass. A load for another diff --git a/packages/ui/src/stores/contextStore.ts b/packages/ui/src/stores/contextStore.ts index ad824f15..7ee4d7c0 100644 --- a/packages/ui/src/stores/contextStore.ts +++ b/packages/ui/src/stores/contextStore.ts @@ -24,8 +24,10 @@ interface ContextState { sessionAgentModelSelections: Map>; - // sessionId → agentName → "providerId/modelId" → variant - sessionAgentModelVariantSelections: Map>>; + // sessionId → agentName → "providerId/modelId" → variant, where `null` is + // an explicit "Default" (send no effort) and a missing entry means the + // inherited default applies. + sessionAgentModelVariantSelections: Map>>; currentAgentContext: Map; @@ -45,8 +47,8 @@ interface ContextActions { saveAgentModelForSession: (sessionId: string, agentName: string, providerId: string, modelId: string) => void; getAgentModelForSession: (sessionId: string, agentName: string) => { providerId: string; modelId: string } | null; - saveAgentModelVariantForSession: (sessionId: string, agentName: string, providerId: string, modelId: string, variant: string | undefined) => void; - getAgentModelVariantForSession: (sessionId: string, agentName: string, providerId: string, modelId: string) => string | undefined; + saveAgentModelVariantForSession: (sessionId: string, agentName: string, providerId: string, modelId: string, variant: string | null | undefined) => void; + getAgentModelVariantForSession: (sessionId: string, agentName: string, providerId: string, modelId: string) => string | null | undefined; getContextUsage: (sessionId: string, contextLimit: number, outputLimit: number, messages: Map) => ContextUsage | null; @@ -145,7 +147,7 @@ export const useContextStore = create()( return agentMap.get(agentName) || null; }, - saveAgentModelVariantForSession: (sessionId: string, agentName: string, providerId: string, modelId: string, variant: string | undefined) => { + saveAgentModelVariantForSession: (sessionId: string, agentName: string, providerId: string, modelId: string, variant: string | null | undefined) => { set((state) => { const newSelections = new Map(state.sessionAgentModelVariantSelections); diff --git a/packages/ui/src/stores/useConfigStore.test.ts b/packages/ui/src/stores/useConfigStore.test.ts index 5ac465ff..2039ae52 100644 --- a/packages/ui/src/stores/useConfigStore.test.ts +++ b/packages/ui/src/stores/useConfigStore.test.ts @@ -544,7 +544,8 @@ describe('useConfigStore provider persistence', () => { useConfigStore.getState().setCurrentVariantOverride('max', 'high'); expect(useConfigStore.getState().cycleCurrentVariant()).toBe(undefined); - expect(useConfigStore.getState().currentVariant).toBe('high'); + // Default is a choice to send no effort, not a way back to the inherited one. + expect(useConfigStore.getState().currentVariant).toBe(undefined); expect(useConfigStore.getState().currentVariantSelection).toEqual({ override: null, inherited: 'high' }); }); @@ -562,7 +563,7 @@ describe('useConfigStore provider persistence', () => { expect(useConfigStore.getState().currentVariantSelection.override).toBe('high'); expect(useConfigStore.getState().cycleCurrentVariant()).toBe(undefined); expect(useConfigStore.getState().currentVariantSelection.override).toBeNull(); - expect(useConfigStore.getState().currentVariant).toBe('high'); + expect(useConfigStore.getState().currentVariant).toBe(undefined); }); test('an unavailable explicit variant cycles back to Default', () => { @@ -576,7 +577,7 @@ describe('useConfigStore provider persistence', () => { }); expect(useConfigStore.getState().cycleCurrentVariant()).toBe(undefined); - expect(useConfigStore.getState().currentVariant).toBe('low'); + expect(useConfigStore.getState().currentVariant).toBe(undefined); expect(useConfigStore.getState().currentVariantSelection.override).toBeNull(); }); @@ -608,6 +609,73 @@ describe('useConfigStore provider persistence', () => { expect(useConfigStore.getState().currentVariant).toBe('medium'); }); + test('an explicit Default effort sends no variant instead of the settings default', () => { + useConfigStore.setState({ + activeDirectoryKey: DIRECTORY, + providers: [provider('openai', 'gpt-5.5', { low: {}, high: {} })], + currentProviderId: 'openai', + currentModelId: 'gpt-5.5', + currentVariant: 'low', + currentVariantSelection: { override: 'low', inherited: 'low' }, + settingsDefaultVariant: 'low', + directoryScoped: {}, + }); + + useConfigStore.getState().setCurrentVariantOverride(null, 'low'); + + expect(useConfigStore.getState().currentVariant).toBe(undefined); + expect(useConfigStore.getState().currentVariantSelection).toEqual({ override: null, inherited: 'low' }); + }); + + test('setAgent keeps a session Default effort instead of restoring the settings default', () => { + const sessionId = 'ses_agent_default_effort'; + useSessionUIStore.setState({ currentSessionId: sessionId }); + useSelectionStore.getState().saveAgentModelForSession(sessionId, 'plan', 'openai', 'gpt-5.5'); + useSelectionStore.getState().saveAgentModelVariantForSession(sessionId, 'plan', 'openai', 'gpt-5.5', null); + useConfigStore.setState({ + activeDirectoryKey: DIRECTORY, + providers: [provider('openai', 'gpt-5.5', { low: {}, high: {} })], + agents: [testAgent('plan')], + settingsDefaultVariant: 'low', + currentProviderId: 'openai', + currentModelId: 'gpt-5.5', + currentVariant: 'low', + currentVariantSelection: { override: undefined, inherited: 'low' }, + directoryScoped: {}, + }); + + useConfigStore.getState().setAgent('plan'); + + const state = useConfigStore.getState(); + expect(state.currentVariant).toBe(undefined); + expect(state.currentVariantSelection).toEqual({ override: null, inherited: 'low' }); + expect(state.directoryScoped[DIRECTORY]?.currentVariant).toBe(undefined); + }); + + test('setAgent reports the same effort through currentVariant and the picker selection', () => { + const sessionId = 'ses_agent_effort_in_sync'; + useSessionUIStore.setState({ currentSessionId: sessionId }); + useSelectionStore.getState().saveAgentModelForSession(sessionId, 'plan', 'openai', 'gpt-5.5'); + useSelectionStore.getState().saveAgentModelVariantForSession(sessionId, 'plan', 'openai', 'gpt-5.5', 'high'); + useConfigStore.setState({ + activeDirectoryKey: DIRECTORY, + providers: [provider('openai', 'gpt-5.5', { low: {}, high: {} })], + agents: [testAgent('plan')], + settingsDefaultVariant: 'low', + currentProviderId: 'openai', + currentModelId: 'gpt-5.5', + currentVariant: 'low', + currentVariantSelection: { override: 'low', inherited: 'low' }, + directoryScoped: {}, + }); + + useConfigStore.getState().setAgent('plan'); + + const state = useConfigStore.getState(); + expect(state.currentVariant).toBe('high'); + expect(state.currentVariantSelection).toEqual({ override: 'high', inherited: 'low' }); + }); + test('setAgent applies settings default variant for a saved session agent model', () => { const sessionId = 'ses_existing_agent_model_default_variant'; useSessionUIStore.setState({ currentSessionId: sessionId }); diff --git a/packages/ui/src/stores/useConfigStore.ts b/packages/ui/src/stores/useConfigStore.ts index e061a64c..a18421b4 100644 --- a/packages/ui/src/stores/useConfigStore.ts +++ b/packages/ui/src/stores/useConfigStore.ts @@ -907,11 +907,28 @@ interface DirectoryScopedConfig { selectionSource?: "auto" | "manual"; } +/** + * The thinking-effort selection, split into what the user picked and what + * applies when they picked nothing: + * + * - `override: string` an effort chosen in the picker + * - `override: null` "Default" chosen in the picker — send no effort + * - `override: undefined` nothing chosen — the inherited default applies + * + * `null` and `undefined` are not interchangeable: collapsing them makes the + * "Default" entry unpickable, because the settings default silently takes + * effect again and the next assistant reply echoes it back as an explicit + * choice. + */ type CurrentVariantSelection = { override: string | null | undefined; inherited: string | undefined; }; +const resolveVariantFromSelection = (selection: CurrentVariantSelection): string | undefined => ( + selection.override === null ? undefined : selection.override ?? selection.inherited +); + /** * Lift the active directory's cached provider/agent snapshot into the top-level * fields the pickers read (`providers`, `agents`, selections), so a cold start @@ -1908,7 +1925,7 @@ export const useConfigStore = create()( setCurrentVariantOverride: (override, inherited) => { set((state) => { - const currentVariant = override ?? inherited; + const currentVariant = resolveVariantFromSelection({ override, inherited }); if ( state.currentVariant === currentVariant && state.currentVariantSelection.override === override @@ -2534,8 +2551,27 @@ export const useConfigStore = create()( if (agentName) { const { currentSessionId } = useSessionUIStore.getState(); - const applyResolvedModelSelection = (providerId: string, modelId: string, variant?: string) => { + // Writes the effort alongside the model, because the two are one + // selection: leaving `currentVariantSelection` behind would let the + // picker show one effort while sends carry another. + const applyResolvedModelSelection = ( + providerId: string, + modelId: string, + variantSelection: CurrentVariantSelection, + ) => { set((state) => { + const variant = resolveVariantFromSelection(variantSelection); + if ( + state.currentProviderId === providerId + && state.currentModelId === modelId + && state.currentVariant === variant + && state.currentVariantSelection.override === variantSelection.override + && state.currentVariantSelection.inherited === variantSelection.inherited + && state.selectionSource === "manual" + ) { + return state; + } + const directoryKey = state.activeDirectoryKey; const baseSnapshot: DirectoryScopedConfig = state.directoryScoped[directoryKey] ?? { providers: state.providers, @@ -2561,6 +2597,7 @@ export const useConfigStore = create()( currentProviderId: providerId, currentModelId: modelId, currentVariant: variant, + currentVariantSelection: variantSelection, selectionSource: "manual", directoryScoped: { ...state.directoryScoped, @@ -2570,16 +2607,24 @@ export const useConfigStore = create()( }); }; - const resolveVariantForModel = ( + const resolveVariantSelectionForModel = ( providerId: string, modelId: string, agentVariant?: string, - ): string | undefined => { + ): CurrentVariantSelection => { const model = providers .find((provider) => provider.id === providerId) ?.models.find((candidate) => candidate.id === modelId) as { variants?: Record } | undefined; const variants = model?.variants; - if (!variants) return undefined; + if (!variants) return { override: undefined, inherited: undefined }; + + const isAvailable = (candidate: string | null | undefined): candidate is string => ( + candidate !== null + && candidate !== undefined + && Object.prototype.hasOwnProperty.call(variants, candidate) + ); + + const inherited = [agentVariant, settingsDefaultVariant].find(isAvailable); const savedVariant = currentSessionId ? useSelectionStore.getState().getAgentModelVariantForSession( @@ -2589,14 +2634,23 @@ export const useConfigStore = create()( modelId, ) : undefined; - - for (const candidate of [savedVariant, agentVariant, settingsDefaultVariant]) { - if (candidate && Object.prototype.hasOwnProperty.call(variants, candidate)) { - return candidate; - } + // `null` is this session's explicit "Default"; it outranks + // the agent and settings defaults just like a named effort. + if (savedVariant === null || isAvailable(savedVariant)) { + return { override: savedVariant, inherited }; } - return undefined; + // While drafting there is no session record to read the choice + // back from, and switching agent is not a change of effort: + // keep the picker's choice for this same model, "Default" + // (an explicit `null`) included. + const liveSelection = get().currentVariantSelection; + const sameModel = get().currentProviderId === providerId && get().currentModelId === modelId; + if (!currentSessionId && sameModel && (liveSelection.override === null || isAvailable(liveSelection.override))) { + return { override: liveSelection.override, inherited }; + } + + return { override: undefined, inherited }; }; const agent = agents.find((candidate) => candidate.name === agentName); @@ -2608,14 +2662,11 @@ export const useConfigStore = create()( if (currentSessionId) { const existingAgentModel = useSelectionStore.getState().getAgentModelForSession(currentSessionId, agentName); if (existingAgentModel && hasProviderModel(providers, existingAgentModel.providerId, existingAgentModel.modelId)) { - const resolvedVariant = resolveVariantForModel(existingAgentModel.providerId, existingAgentModel.modelId, agent?.variant); - if ( - currentProviderId !== existingAgentModel.providerId - || currentModelId !== existingAgentModel.modelId - || get().currentVariant !== resolvedVariant - ) { - applyResolvedModelSelection(existingAgentModel.providerId, existingAgentModel.modelId, resolvedVariant); - } + applyResolvedModelSelection( + existingAgentModel.providerId, + existingAgentModel.modelId, + resolveVariantSelectionForModel(existingAgentModel.providerId, existingAgentModel.modelId, agent?.variant), + ); return; } } @@ -2628,7 +2679,7 @@ export const useConfigStore = create()( const agentModel = agentProvider?.models.find((model) => model.id === modelID); if (agentModel) { - applyResolvedModelSelection(providerID, modelID, resolveVariantForModel(providerID, modelID, agent?.variant)); + applyResolvedModelSelection(providerID, modelID, resolveVariantSelectionForModel(providerID, modelID, agent?.variant)); return; } } @@ -2660,7 +2711,7 @@ export const useConfigStore = create()( if (parsed) { const settingsProvider = providers.find((p) => p.id === parsed.providerId); if (settingsProvider?.models.some((m) => m.id === parsed.modelId)) { - applyResolvedModelSelection(parsed.providerId, parsed.modelId, resolveVariantForModel(parsed.providerId, parsed.modelId, agent?.variant)); + applyResolvedModelSelection(parsed.providerId, parsed.modelId, resolveVariantSelectionForModel(parsed.providerId, parsed.modelId, agent?.variant)); return; } } diff --git a/packages/ui/src/sync/selection-store.ts b/packages/ui/src/sync/selection-store.ts index a8c1cb21..6762ef05 100644 --- a/packages/ui/src/sync/selection-store.ts +++ b/packages/ui/src/sync/selection-store.ts @@ -29,16 +29,21 @@ export type SelectionState = { getSessionAgentSelection: (sessionId: string) => string | null saveAgentModelForSession: (sessionId: string, agentName: string, providerId: string, modelId: string) => void getAgentModelForSession: (sessionId: string, agentName: string) => { providerId: string; modelId: string } | null - saveAgentModelVariantForSession: (sessionId: string, agentName: string, providerId: string, modelId: string, variant: string | undefined) => void - getAgentModelVariantForSession: (sessionId: string, agentName: string, providerId: string, modelId: string) => string | undefined + /** + * `variant` is the effort chosen for this agent/model in this session: + * a name, `null` for an explicit "Default" (send no effort), or `undefined` + * to forget the choice so the inherited default applies again. + */ + saveAgentModelVariantForSession: (sessionId: string, agentName: string, providerId: string, modelId: string, variant: string | null | undefined) => void + getAgentModelVariantForSession: (sessionId: string, agentName: string, providerId: string, modelId: string) => string | null | undefined } const isPersistedSelectionState = (state: unknown): state is PersistedSelectionState => ( typeof state === "object" && state !== null ) -// In-memory variant storage (not persisted) -const agentModelVariantSelections = new Map>>() +// In-memory variant storage (not persisted). `null` is an explicit "Default". +const agentModelVariantSelections = new Map>>() // Maximum number of sessions to persist to local storage to prevent unbounded growth const MAX_PERSISTED_SESSIONS = 150 @@ -91,20 +96,21 @@ export const useSelectionStore = create()( saveAgentModelVariantForSession: (sessionId, agentName, providerId, modelId, variant) => { const key = `${providerId}/${modelId}` + const clears = variant === undefined let agentMap = agentModelVariantSelections.get(sessionId) - if (!agentMap && variant) { + if (!agentMap && !clears) { agentMap = new Map() agentModelVariantSelections.set(sessionId, agentMap) } if (!agentMap) return let modelMap = agentMap.get(agentName) - if (!modelMap && variant) { + if (!modelMap && !clears) { modelMap = new Map() agentMap.set(agentName, modelMap) } if (!modelMap) return - if (!variant) { + if (clears) { modelMap.delete(key) if (modelMap.size === 0) { agentMap.delete(agentName) diff --git a/packages/ui/src/sync/session-ui-store.ts b/packages/ui/src/sync/session-ui-store.ts index bda558e0..6648550b 100644 --- a/packages/ui/src/sync/session-ui-store.ts +++ b/packages/ui/src/sync/session-ui-store.ts @@ -852,10 +852,13 @@ export async function materializeOpenDraftSession(selection: { }) const effectiveDraftAgent = trimmedAgent ?? configState.currentAgentName + // An explicit "Default" (`null`) is carried over as-is. Flattening it to + // `undefined` here would leave the new session with no recorded choice, and + // the settings default effort would take the picker back over. const variantOverride = configState.currentProviderId === selection.providerID && configState.currentModelId === selection.modelID && configState.currentAgentName === effectiveDraftAgent - ? configState.currentVariantSelection.override ?? undefined + ? configState.currentVariantSelection.override : selection.variant useSelectionStore.getState().saveSessionModelSelection(created.id, selection.providerID, selection.modelID)