Revert "revert(ui): restore inherited semantics for Default thinking effort"
This reverts commit 35875e818c.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -24,8 +24,10 @@ interface ContextState {
|
||||
|
||||
sessionAgentModelSelections: Map<string, Map<string, { providerId: string; modelId: string }>>;
|
||||
|
||||
// sessionId → agentName → "providerId/modelId" → variant
|
||||
sessionAgentModelVariantSelections: Map<string, Map<string, Map<string, string>>>;
|
||||
// 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<string, Map<string, Map<string, string | null>>>;
|
||||
|
||||
currentAgentContext: Map<string, string>;
|
||||
|
||||
@@ -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<string, { info: any; parts: any[] }[]>) => ContextUsage | null;
|
||||
@@ -145,7 +147,7 @@ export const useContextStore = create<ContextStore>()(
|
||||
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);
|
||||
|
||||
|
||||
@@ -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 });
|
||||
|
||||
@@ -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<ConfigStore>()(
|
||||
|
||||
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<ConfigStore>()(
|
||||
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<ConfigStore>()(
|
||||
currentProviderId: providerId,
|
||||
currentModelId: modelId,
|
||||
currentVariant: variant,
|
||||
currentVariantSelection: variantSelection,
|
||||
selectionSource: "manual",
|
||||
directoryScoped: {
|
||||
...state.directoryScoped,
|
||||
@@ -2570,16 +2607,24 @@ export const useConfigStore = create<ConfigStore>()(
|
||||
});
|
||||
};
|
||||
|
||||
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<string, unknown> } | 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<ConfigStore>()(
|
||||
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<ConfigStore>()(
|
||||
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<ConfigStore>()(
|
||||
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<ConfigStore>()(
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user