fix: preserve settings default thinking variant when switching agents (#1639)
* fix: preserve settings default thinking variant when switching agents When a user sets a default thinking variant (e.g. 'high') in settings and switches between plan and build agents in a session, the variant was reset to 'default' (undefined) instead of respecting the settings default. Root cause: two code paths failed to fall back to settingsDefaultVariant: 1. ModelControls variant sync effect: when no per-session+agent+model variant was saved, the effect set currentVariant to undefined instead of falling back to settingsDefaultVariant. 2. setAgent in useConfigStore: when the target agent had a configured model, the variant was always passed as undefined to applyResolvedModelSelection, ignoring both the saved per-session variant and the settings default. Fix both paths to resolve variants in priority order: saved variant > settingsDefaultVariant > undefined. * fix: preserve agent variant fallback order * fix: apply historical session variant on restore --------- Co-authored-by: Bohdan Triapitsyn <artmore@protonmail.com>
This commit is contained in:
committed by
GitHub
co-authored by
Bohdan Triapitsyn
parent
b863a4a83a
commit
3f4ad2a3e8
@@ -831,9 +831,14 @@ export const ModelControls: React.FC<ModelControlsProps> = ({
|
|||||||
setAgent(latestLoadedUserChoice.agent);
|
setAgent(latestLoadedUserChoice.agent);
|
||||||
}
|
}
|
||||||
|
|
||||||
const applyResult = tryApplyModelSelection(
|
const historicalVariant = latestLoadedUserChoice.variant
|
||||||
|
&& getModelVariantOptions(latestLoadedUserChoice.providerID, latestLoadedUserChoice.modelID).includes(latestLoadedUserChoice.variant)
|
||||||
|
? latestLoadedUserChoice.variant
|
||||||
|
: undefined;
|
||||||
|
const applyResult = applyModelSelectionWithVariant(
|
||||||
latestLoadedUserChoice.providerID,
|
latestLoadedUserChoice.providerID,
|
||||||
latestLoadedUserChoice.modelID,
|
latestLoadedUserChoice.modelID,
|
||||||
|
historicalVariant,
|
||||||
latestLoadedUserChoice.agent || currentAgentName || undefined,
|
latestLoadedUserChoice.agent || currentAgentName || undefined,
|
||||||
);
|
);
|
||||||
if (applyResult !== 'applied') {
|
if (applyResult !== 'applied') {
|
||||||
@@ -847,7 +852,7 @@ export const ModelControls: React.FC<ModelControlsProps> = ({
|
|||||||
latestLoadedUserChoice.agent,
|
latestLoadedUserChoice.agent,
|
||||||
latestLoadedUserChoice.providerID,
|
latestLoadedUserChoice.providerID,
|
||||||
latestLoadedUserChoice.modelID,
|
latestLoadedUserChoice.modelID,
|
||||||
latestLoadedUserChoice.variant,
|
historicalVariant,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
saveSessionModelSelection(currentSessionId, latestLoadedUserChoice.providerID, latestLoadedUserChoice.modelID);
|
saveSessionModelSelection(currentSessionId, latestLoadedUserChoice.providerID, latestLoadedUserChoice.modelID);
|
||||||
@@ -861,7 +866,8 @@ export const ModelControls: React.FC<ModelControlsProps> = ({
|
|||||||
hasRenderableCurrentSessionSnapshot,
|
hasRenderableCurrentSessionSnapshot,
|
||||||
latestLoadedUserChoice,
|
latestLoadedUserChoice,
|
||||||
setAgent,
|
setAgent,
|
||||||
tryApplyModelSelection,
|
applyModelSelectionWithVariant,
|
||||||
|
getModelVariantOptions,
|
||||||
saveSessionAgentSelection,
|
saveSessionAgentSelection,
|
||||||
saveAgentModelVariantForSession,
|
saveAgentModelVariantForSession,
|
||||||
saveSessionModelSelection,
|
saveSessionModelSelection,
|
||||||
@@ -1144,7 +1150,9 @@ export const ModelControls: React.FC<ModelControlsProps> = ({
|
|||||||
|
|
||||||
const resolvedSaved = savedVariant && availableVariants.includes(savedVariant)
|
const resolvedSaved = savedVariant && availableVariants.includes(savedVariant)
|
||||||
? savedVariant
|
? savedVariant
|
||||||
: undefined;
|
: settingsDefaultVariant && availableVariants.includes(settingsDefaultVariant)
|
||||||
|
? settingsDefaultVariant
|
||||||
|
: undefined;
|
||||||
|
|
||||||
setCurrentVariant(resolvedSaved);
|
setCurrentVariant(resolvedSaved);
|
||||||
manualVariantSelectionRef.current = false;
|
manualVariantSelectionRef.current = false;
|
||||||
|
|||||||
@@ -216,6 +216,8 @@ mock.module('@/lib/configSync', () => ({
|
|||||||
|
|
||||||
const { useConfigStore } = await import('./useConfigStore');
|
const { useConfigStore } = await import('./useConfigStore');
|
||||||
const { emitSyncConfigChanged, setSyncRefs } = await import('@/sync/sync-refs');
|
const { emitSyncConfigChanged, setSyncRefs } = await import('@/sync/sync-refs');
|
||||||
|
const { useSelectionStore } = await import('@/sync/selection-store');
|
||||||
|
const { useSessionUIStore } = await import('@/sync/session-ui-store');
|
||||||
|
|
||||||
describe('useConfigStore provider persistence', () => {
|
describe('useConfigStore provider persistence', () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
@@ -235,6 +237,13 @@ describe('useConfigStore provider persistence', () => {
|
|||||||
withDirectoryCalls = [];
|
withDirectoryCalls = [];
|
||||||
currentFetchDirectory = DIRECTORY;
|
currentFetchDirectory = DIRECTORY;
|
||||||
setSyncRefs({} as never, { children: new Map(), getState: () => undefined } as never, DIRECTORY);
|
setSyncRefs({} as never, { children: new Map(), getState: () => undefined } as never, DIRECTORY);
|
||||||
|
useSelectionStore.setState({
|
||||||
|
sessionModelSelections: new Map(),
|
||||||
|
sessionAgentSelections: new Map(),
|
||||||
|
sessionAgentModelSelections: new Map(),
|
||||||
|
lastUsedProvider: null,
|
||||||
|
});
|
||||||
|
useSessionUIStore.setState({ currentSessionId: null });
|
||||||
useConfigStore.setState({
|
useConfigStore.setState({
|
||||||
activeDirectoryKey: DIRECTORY,
|
activeDirectoryKey: DIRECTORY,
|
||||||
directoryScoped: {},
|
directoryScoped: {},
|
||||||
@@ -380,6 +389,79 @@ describe('useConfigStore provider persistence', () => {
|
|||||||
expect(state.currentVariant).toBe('fast');
|
expect(state.currentVariant).toBe('fast');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('setAgent applies settings default variant for an agent configured model', () => {
|
||||||
|
useSessionUIStore.setState({ currentSessionId: 'ses_agent_default_variant' });
|
||||||
|
useConfigStore.setState({
|
||||||
|
activeDirectoryKey: DIRECTORY,
|
||||||
|
providers: [provider('openai', 'gpt-5.5', { low: {}, high: {} })],
|
||||||
|
agents: [testAgent('plan', { model: { providerID: 'openai', modelID: 'gpt-5.5' } })],
|
||||||
|
settingsDefaultVariant: 'high',
|
||||||
|
currentProviderId: 'openai',
|
||||||
|
currentModelId: 'gpt-5.5',
|
||||||
|
currentVariant: undefined,
|
||||||
|
directoryScoped: {},
|
||||||
|
});
|
||||||
|
|
||||||
|
useConfigStore.getState().setAgent('plan');
|
||||||
|
|
||||||
|
const state = useConfigStore.getState();
|
||||||
|
expect(state.currentProviderId).toBe('openai');
|
||||||
|
expect(state.currentModelId).toBe('gpt-5.5');
|
||||||
|
expect(state.currentVariant).toBe('high');
|
||||||
|
expect(state.directoryScoped[DIRECTORY]?.currentVariant).toBe('high');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('setAgent prefers saved and agent variants before settings default', () => {
|
||||||
|
const sessionId = 'ses_agent_saved_variant';
|
||||||
|
useSessionUIStore.setState({ currentSessionId: sessionId });
|
||||||
|
useSelectionStore.getState().saveAgentModelVariantForSession(sessionId, 'plan', 'openai', 'gpt-5.5', 'low');
|
||||||
|
useConfigStore.setState({
|
||||||
|
activeDirectoryKey: DIRECTORY,
|
||||||
|
providers: [provider('openai', 'gpt-5.5', { low: {}, medium: {}, high: {} })],
|
||||||
|
agents: [testAgent('plan', {
|
||||||
|
model: { providerID: 'openai', modelID: 'gpt-5.5' },
|
||||||
|
variant: 'medium',
|
||||||
|
})],
|
||||||
|
settingsDefaultVariant: 'high',
|
||||||
|
currentProviderId: 'openai',
|
||||||
|
currentModelId: 'gpt-5.5',
|
||||||
|
currentVariant: undefined,
|
||||||
|
directoryScoped: {},
|
||||||
|
});
|
||||||
|
|
||||||
|
useConfigStore.getState().setAgent('plan');
|
||||||
|
expect(useConfigStore.getState().currentVariant).toBe('low');
|
||||||
|
|
||||||
|
useSelectionStore.getState().saveAgentModelVariantForSession(sessionId, 'plan', 'openai', 'gpt-5.5', undefined);
|
||||||
|
useConfigStore.setState({ currentVariant: undefined, directoryScoped: {} });
|
||||||
|
|
||||||
|
useConfigStore.getState().setAgent('plan');
|
||||||
|
expect(useConfigStore.getState().currentVariant).toBe('medium');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('setAgent applies settings default variant for a saved session agent model', () => {
|
||||||
|
const sessionId = 'ses_existing_agent_model_default_variant';
|
||||||
|
useSessionUIStore.setState({ currentSessionId: sessionId });
|
||||||
|
useSelectionStore.getState().saveAgentModelForSession(sessionId, 'plan', 'openai', 'gpt-5.5');
|
||||||
|
useConfigStore.setState({
|
||||||
|
activeDirectoryKey: DIRECTORY,
|
||||||
|
providers: [provider('openai', 'gpt-5.5', { low: {}, high: {} })],
|
||||||
|
agents: [testAgent('plan')],
|
||||||
|
settingsDefaultVariant: 'high',
|
||||||
|
currentProviderId: 'other',
|
||||||
|
currentModelId: 'other-model',
|
||||||
|
currentVariant: undefined,
|
||||||
|
directoryScoped: {},
|
||||||
|
});
|
||||||
|
|
||||||
|
useConfigStore.getState().setAgent('plan');
|
||||||
|
|
||||||
|
const state = useConfigStore.getState();
|
||||||
|
expect(state.currentProviderId).toBe('openai');
|
||||||
|
expect(state.currentModelId).toBe('gpt-5.5');
|
||||||
|
expect(state.currentVariant).toBe('high');
|
||||||
|
});
|
||||||
|
|
||||||
test('loadAgents does not fetch OpenCode config directly', async () => {
|
test('loadAgents does not fetch OpenCode config directly', async () => {
|
||||||
useConfigStore.setState({
|
useConfigStore.setState({
|
||||||
activeDirectoryKey: DIRECTORY,
|
activeDirectoryKey: DIRECTORY,
|
||||||
|
|||||||
@@ -2437,6 +2437,35 @@ export const useConfigStore = create<ConfigStore>()(
|
|||||||
});
|
});
|
||||||
};
|
};
|
||||||
|
|
||||||
|
const resolveVariantForModel = (
|
||||||
|
providerId: string,
|
||||||
|
modelId: string,
|
||||||
|
agentVariant?: string,
|
||||||
|
): string | undefined => {
|
||||||
|
const model = providers
|
||||||
|
.find((provider) => provider.id === providerId)
|
||||||
|
?.models.find((candidate) => candidate.id === modelId) as { variants?: Record<string, unknown> } | undefined;
|
||||||
|
const variants = model?.variants;
|
||||||
|
if (!variants) return undefined;
|
||||||
|
|
||||||
|
const savedVariant = currentSessionId
|
||||||
|
? useSelectionStore.getState().getAgentModelVariantForSession(
|
||||||
|
currentSessionId,
|
||||||
|
agentName,
|
||||||
|
providerId,
|
||||||
|
modelId,
|
||||||
|
)
|
||||||
|
: undefined;
|
||||||
|
|
||||||
|
for (const candidate of [savedVariant, agentVariant, settingsDefaultVariant]) {
|
||||||
|
if (candidate && Object.prototype.hasOwnProperty.call(variants, candidate)) {
|
||||||
|
return candidate;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return undefined;
|
||||||
|
};
|
||||||
|
|
||||||
// Prefer the selected agent's configured model when switching agents.
|
// Prefer the selected agent's configured model when switching agents.
|
||||||
const agent = agents.find((candidate) => candidate.name === agentName);
|
const agent = agents.find((candidate) => candidate.name === agentName);
|
||||||
const agentModelSelection = agent?.model;
|
const agentModelSelection = agent?.model;
|
||||||
@@ -2446,7 +2475,7 @@ export const useConfigStore = create<ConfigStore>()(
|
|||||||
const agentModel = agentProvider?.models.find((model) => model.id === modelID);
|
const agentModel = agentProvider?.models.find((model) => model.id === modelID);
|
||||||
|
|
||||||
if (agentModel) {
|
if (agentModel) {
|
||||||
applyResolvedModelSelection(providerID, modelID, undefined);
|
applyResolvedModelSelection(providerID, modelID, resolveVariantForModel(providerID, modelID, agent?.variant));
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -2454,18 +2483,13 @@ export const useConfigStore = create<ConfigStore>()(
|
|||||||
if (currentSessionId) {
|
if (currentSessionId) {
|
||||||
const existingAgentModel = useSelectionStore.getState().getAgentModelForSession(currentSessionId, agentName);
|
const existingAgentModel = useSelectionStore.getState().getAgentModelForSession(currentSessionId, agentName);
|
||||||
if (existingAgentModel && hasProviderModel(providers, existingAgentModel.providerId, existingAgentModel.modelId)) {
|
if (existingAgentModel && hasProviderModel(providers, existingAgentModel.providerId, existingAgentModel.modelId)) {
|
||||||
const savedVariant = useSelectionStore.getState().getAgentModelVariantForSession(
|
const resolvedVariant = resolveVariantForModel(existingAgentModel.providerId, existingAgentModel.modelId, agent?.variant);
|
||||||
currentSessionId,
|
|
||||||
agentName,
|
|
||||||
existingAgentModel.providerId,
|
|
||||||
existingAgentModel.modelId,
|
|
||||||
);
|
|
||||||
if (
|
if (
|
||||||
currentProviderId !== existingAgentModel.providerId
|
currentProviderId !== existingAgentModel.providerId
|
||||||
|| currentModelId !== existingAgentModel.modelId
|
|| currentModelId !== existingAgentModel.modelId
|
||||||
|| get().currentVariant !== savedVariant
|
|| get().currentVariant !== resolvedVariant
|
||||||
) {
|
) {
|
||||||
applyResolvedModelSelection(existingAgentModel.providerId, existingAgentModel.modelId, savedVariant);
|
applyResolvedModelSelection(existingAgentModel.providerId, existingAgentModel.modelId, resolvedVariant);
|
||||||
}
|
}
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -2477,16 +2501,7 @@ export const useConfigStore = create<ConfigStore>()(
|
|||||||
if (parsed) {
|
if (parsed) {
|
||||||
const settingsProvider = providers.find((p) => p.id === parsed.providerId);
|
const settingsProvider = providers.find((p) => p.id === parsed.providerId);
|
||||||
if (settingsProvider?.models.some((m) => m.id === parsed.modelId)) {
|
if (settingsProvider?.models.some((m) => m.id === parsed.modelId)) {
|
||||||
let nextVariant: string | undefined;
|
applyResolvedModelSelection(parsed.providerId, parsed.modelId, resolveVariantForModel(parsed.providerId, parsed.modelId, agent?.variant));
|
||||||
if (settingsDefaultVariant) {
|
|
||||||
const model = settingsProvider.models.find((m) => m.id === parsed.modelId) as { variants?: Record<string, unknown> } | undefined;
|
|
||||||
const variants = model?.variants;
|
|
||||||
if (variants && Object.prototype.hasOwnProperty.call(variants, settingsDefaultVariant)) {
|
|
||||||
nextVariant = settingsDefaultVariant;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
applyResolvedModelSelection(parsed.providerId, parsed.modelId, nextVariant);
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user