From 025cd3d46cc220da5b8745709d031bfedf08cc86 Mon Sep 17 00:00:00 2001 From: Iuliia Ivashko Date: Fri, 4 Sep 2026 17:32:40 +0300 Subject: [PATCH] fix(ui): let restore paths report no effort without pinning Default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `commitVariantSelectionForModel` turned every `undefined` into an explicit `Default` (`null`), but it serves two kinds of caller. The picker means "the user chose Default"; the history and manual-override restores mean "nothing was found". Restoring a session therefore recorded a choice nobody made, and `resolveModelVariantSelection` collapsed that `null` back to `undefined`, so the next restore recorded it again. Because an explicit `Default` outranks the agent and settings defaults by design, the session latched onto `Default` and the concrete effort its own history carried could not come back. Move the decision to the callers: the four picker paths pass `variant ?? null`, the restore paths pass their result through, and the resolver returns the selection store's three states instead of two. The follow-up write in the history restore goes with it — the apply above it already recorded the same agent and model, and a second write could only disagree. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EZuVVgziiLjD81W5vaxdH2 --- .../ui/src/components/chat/ModelControls.tsx | 63 ++++++++++++------- packages/ui/src/stores/DOCUMENTATION.md | 7 +++ 2 files changed, 47 insertions(+), 23 deletions(-) diff --git a/packages/ui/src/components/chat/ModelControls.tsx b/packages/ui/src/components/chat/ModelControls.tsx index 880b9475..c934bfef 100644 --- a/packages/ui/src/components/chat/ModelControls.tsx +++ b/packages/ui/src/components/chat/ModelControls.tsx @@ -725,7 +725,14 @@ export const ModelControls: React.FC = ({ return candidates.find((candidate) => candidate !== undefined && variantOptions.includes(candidate)); }, [agents, currentAgentName, currentModelId, currentProviderId, currentSessionId, currentVariantSelection, effectiveCurrentVariant, getModelVariantOptions, settingsDefaultVariant, uiAgentName]); - const resolveModelVariantSelection = React.useCallback((providerId: string, modelId: string) => { + /** + * The session's recorded choice for this model, in the selection store's + * three states: an effort name, `null` for an explicit "Default", and + * `undefined` for no choice at all. Collapsing `null` into `undefined` here + * would hand a real "Default" back to the callers as "nothing chosen", and + * they would re-record it as a choice on the next write. + */ + const resolveModelVariantSelection = React.useCallback((providerId: string, modelId: string): string | null | undefined => { const variantOptions = getModelVariantOptions(providerId, modelId); if (variantOptions.length === 0) { return undefined; @@ -736,7 +743,7 @@ export const ModelControls: React.FC = ({ const savedVariant = getAgentModelVariantForSession(currentSessionId, effectiveAgentName, providerId, modelId); // An explicit "Default" is a choice: it stops the fallbacks below. if (savedVariant === null) { - return undefined; + return null; } if (savedVariant && variantOptions.includes(savedVariant)) { return savedVariant; @@ -770,7 +777,16 @@ export const ModelControls: React.FC = ({ return liveConfigAgentName || currentAgentName; }, [currentAgentName, currentSessionId]); - const commitVariantSelectionForModel = React.useCallback((providerId: string, modelId: string, variant: string | undefined, agentNameOverride?: string | null) => { + /** + * Records `variant` as this session's effort for the model, in the same + * three states the selection store defines: an effort name, `null` for an + * explicit "Default", `undefined` for no choice so the inherited effort + * applies. Callers decide which one they mean — the picker turns its own + * "Default" into `null`, while restore paths pass `undefined` through when + * they found nothing, because "the history carries no effort" is not the + * user having chosen "Default". + */ + const commitVariantSelectionForModel = React.useCallback((providerId: string, modelId: string, variant: string | null | undefined, agentNameOverride?: string | null) => { const variantOptions = getModelVariantOptions(providerId, modelId); if (variantOptions.length === 0) { manualVariantSelectionRef.current = false; @@ -780,16 +796,14 @@ export const ModelControls: React.FC = ({ manualVariantSelectionRef.current = true; setCurrentVariantOverride( - variant ?? null, + variant, resolveInheritedVariantForModel(providerId, modelId, agentNameOverride), ); - addRecentEffort(providerId, modelId, variant); + addRecentEffort(providerId, modelId, variant ?? undefined); const effectiveAgentName = agentNameOverride ?? resolveLiveAgentName(); if (currentSessionId && effectiveAgentName) { - // `null`, not `undefined`: picking "Default" is a choice to record, - // not the absence of one. - saveAgentModelVariantForSession(currentSessionId, effectiveAgentName, providerId, modelId, variant ?? null); + saveAgentModelVariantForSession(currentSessionId, effectiveAgentName, providerId, modelId, variant); } }, [ addRecentEffort, @@ -802,7 +816,7 @@ export const ModelControls: React.FC = ({ setCurrentVariantOverride, ]); - const applyModelSelectionWithVariant = React.useCallback((providerId: string, modelId: string, variant: string | undefined, agentNameOverride?: string | null) => { + const applyModelSelectionWithVariant = React.useCallback((providerId: string, modelId: string, variant: string | null | undefined, agentNameOverride?: string | null) => { const effectiveAgentName = agentNameOverride ?? resolveLiveAgentName() ?? undefined; const result = tryApplyModelSelection(providerId, modelId, effectiveAgentName); if (result !== 'applied') { @@ -874,15 +888,11 @@ export const ModelControls: React.FC = ({ return; } + // The effort is not written again here: `applyModelSelectionWithVariant` + // above already recorded `historicalVariant` for this same agent and + // model, and a second write can only disagree with the first. if (latestLoadedUserChoice.agent) { saveSessionAgentSelection(currentSessionId, latestLoadedUserChoice.agent); - saveAgentModelVariantForSession( - currentSessionId, - latestLoadedUserChoice.agent, - latestLoadedUserChoice.providerID, - latestLoadedUserChoice.modelID, - historicalVariant, - ); } saveSessionModelSelection(currentSessionId, latestLoadedUserChoice.providerID, latestLoadedUserChoice.modelID); latestLoadedUserChoiceRestoreRef.current = restoreKey; @@ -900,7 +910,6 @@ export const ModelControls: React.FC = ({ getSessionModelSelection, resolveModelVariantSelection, saveSessionAgentSelection, - saveAgentModelVariantForSession, saveSessionModelSelection, ]); @@ -1184,7 +1193,9 @@ export const ModelControls: React.FC = ({ const handleVariantSelect = React.useCallback((variant: string | undefined) => { if (currentProviderId && currentModelId) { - commitVariantSelectionForModel(currentProviderId, currentModelId, variant); + // Picked in the effort menu, so no effort means the user picked + // "Default" — a choice, recorded as `null`. + commitVariantSelectionForModel(currentProviderId, currentModelId, variant ?? null); } }, [commitVariantSelectionForModel, currentModelId, currentProviderId]); @@ -1246,8 +1257,10 @@ export const ModelControls: React.FC = ({ ) => { try { const effectiveAgentName = options?.agentName ?? resolveLiveAgentName() ?? undefined; + // `applyVariant` is only set when the user adjusted the effort in + // the model picker, so no effort means an explicit "Default". const result = options?.applyVariant - ? applyModelSelectionWithVariant(providerId, modelId, options.variant, effectiveAgentName) + ? applyModelSelectionWithVariant(providerId, modelId, options.variant ?? null, effectiveAgentName) : tryApplyModelSelection(providerId, modelId, effectiveAgentName); if (result !== 'applied') { if (result === 'provider-missing') { @@ -1604,8 +1617,10 @@ export const ModelControls: React.FC = ({ } } - const handleMobileModelApply = (providerId: string, modelId: string, variant: string | undefined) => { - const result = applyModelSelectionWithVariant(providerId, modelId, variant); + const handleMobileModelApply = (providerId: string, modelId: string, variant: string | null | undefined) => { + // Chosen in the mobile model sheet, and the row already showed this + // effort: no effort there means the user is applying "Default". + const result = applyModelSelectionWithVariant(providerId, modelId, variant ?? null); if (result !== 'applied') { if (result === 'provider-missing') { console.error('[ModelControls] Provider not available for selection:', providerId); @@ -1642,7 +1657,8 @@ export const ModelControls: React.FC = ({ const variantOptions = getModelVariantOptions(providerId, modelId); const hasVariants = variantOptions.length > 0; const resolvedVariant = resolveModelVariantSelection(providerId, modelId); - const variantLabel = hasVariants ? formatEffortLabel(resolvedVariant) : null; + // Both an explicit "Default" and no choice at all read as "Default". + const variantLabel = hasVariants ? formatEffortLabel(resolvedVariant ?? undefined) : null; const isExpanded = expandedMobileModelKey === rowKey; const inlineVariantOptions = [undefined, ...variantOptions].slice(0, MAX_INLINE_MOBILE_VARIANT_OPTIONS); const hasVariantOverflow = inlineVariantOptions.length < variantOptions.length + 1; @@ -1947,7 +1963,8 @@ export const ModelControls: React.FC = ({ }; const handleSelect = (variant: string | undefined) => { - const result = applyModelSelectionWithVariant(targetProviderId, targetModelId, variant); + // Chosen in the mobile effort panel: no effort means "Default". + const result = applyModelSelectionWithVariant(targetProviderId, targetModelId, variant ?? null); if (result !== 'applied') { return; } diff --git a/packages/ui/src/stores/DOCUMENTATION.md b/packages/ui/src/stores/DOCUMENTATION.md index 1cb4780d..f92f14ea 100644 --- a/packages/ui/src/stores/DOCUMENTATION.md +++ b/packages/ui/src/stores/DOCUMENTATION.md @@ -232,6 +232,13 @@ 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. +Only a place where the user chose may write `null`. Restore paths — message +history, a preserved manual override — pass their own "found nothing" through +as `undefined`, because a session whose history carries no effort is not a +session where `Default` was picked. A restore that manufactures `null` latches +the session onto `Default`: `null` outranks the agent and settings defaults by +design, so the concrete effort it displaced can never come back. + 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.