-
Notifications
You must be signed in to change notification settings - Fork 415
fix(web): auto-enable default thinking effort when switching to an effort-capable model #1475
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@moonshot-ai/kimi-code": patch | ||
| --- | ||
|
|
||
| web: Auto-enable the default thinking effort when switching to a model that supports effort levels in the web UI. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,7 @@ import { ref, type ComputedRef } from 'vue'; | |
| import { getKimiWebApi } from '../../api'; | ||
| import type { AppMessage, AppModel, AppProvider, AppSession, AppSkill, ThinkingLevel } from '../../api/types'; | ||
| import { safeGetString, safeSetString, STORAGE_KEYS } from '../../lib/storage'; | ||
| import { coerceThinkingForModel } from '../../lib/modelThinking'; | ||
| import { coerceThinkingForModel, thinkingLevelForModelSwitch } from '../../lib/modelThinking'; | ||
| import type { ActivityState } from '../../types'; | ||
| import type { ExtendedState } from '../useKimiWebClient'; | ||
|
|
||
|
|
@@ -184,8 +184,11 @@ export function useModelProviderState( | |
| */ | ||
| async function setModel(modelId: string): Promise<void> { | ||
| const sid = rawState.activeSessionId; | ||
| const nextThinking = coerceThinkingForModel(modelById(modelId), rawState.thinking); | ||
| const targetModel = modelById(modelId); | ||
| const prevThinking = rawState.thinking; | ||
| const prevModel = sid === undefined ? undefined : rawState.sessions.find((s) => s.id === sid)?.model; | ||
| const isSwitch = prevModel !== modelId; | ||
|
Comment on lines
+189
to
+190
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Here Useful? React with 👍 / 👎. |
||
| const nextThinking = thinkingLevelForModelSwitch(targetModel, prevThinking, isSwitch); | ||
| if (!sid) { | ||
| // New-session draft (onboarding composer): no backend session to update. | ||
| // Remember the pick — startSessionAndSendPrompt applies it at create time. | ||
|
|
@@ -195,7 +198,6 @@ export function useModelProviderState( | |
| } | ||
| // Optimistic: show the chosen model immediately, but remember the previous | ||
| // one so we can roll back if the switch never reaches the daemon. | ||
| const prevModel = rawState.sessions.find((s) => s.id === sid)?.model; | ||
| updateSession(sid, (s) => ({ ...s, model: modelId })); | ||
| if (nextThinking !== prevThinking) { | ||
| rawState.thinking = nextThinking; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,186 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
| import { | ||
| coerceThinkingForModel, | ||
| commitLevel, | ||
| defaultThinkingLevelFor, | ||
| effortLabel, | ||
| isThinkingOn, | ||
| modelThinkingAvailability, | ||
| segmentsFor, | ||
| thinkingLevelForModelSwitch, | ||
| } from './modelThinking'; | ||
| import type { ModelThinkingInfo } from './modelThinking'; | ||
|
|
||
| function model(partial: ModelThinkingInfo): ModelThinkingInfo { | ||
| return partial; | ||
| } | ||
|
|
||
| describe('modelThinking', () => { | ||
| describe('modelThinkingAvailability', () => { | ||
| it('defaults to toggle when model is unknown', () => { | ||
| expect(modelThinkingAvailability(undefined)).toBe('toggle'); | ||
| }); | ||
|
|
||
| it('detects always_thinking capability', () => { | ||
| expect(modelThinkingAvailability(model({ capabilities: ['always_thinking'] }))).toBe('always-on'); | ||
| }); | ||
|
|
||
| it('detects thinking capability', () => { | ||
| expect(modelThinkingAvailability(model({ capabilities: ['thinking'] }))).toBe('toggle'); | ||
| }); | ||
|
|
||
| it('detects adaptive thinking', () => { | ||
| expect(modelThinkingAvailability(model({ adaptiveThinking: true }))).toBe('toggle'); | ||
| }); | ||
|
|
||
| it('marks models without thinking support as unsupported', () => { | ||
| expect(modelThinkingAvailability(model({ capabilities: ['vision'] }))).toBe('unsupported'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('defaultThinkingLevelFor', () => { | ||
| it('returns off for unsupported models', () => { | ||
| expect(defaultThinkingLevelFor(model({ capabilities: [] }))).toBe('off'); | ||
| }); | ||
|
|
||
| it('returns the declared default effort for effort models', () => { | ||
| expect(defaultThinkingLevelFor(model({ capabilities: ['thinking'], supportEfforts: ['low', 'high', 'max'], defaultEffort: 'high' }))).toBe('high'); | ||
| }); | ||
|
|
||
| it('falls back to the middle effort when no default is declared', () => { | ||
| expect(defaultThinkingLevelFor(model({ capabilities: ['thinking'], supportEfforts: ['low', 'high', 'max'] }))).toBe('high'); | ||
| expect(defaultThinkingLevelFor(model({ capabilities: ['thinking'], supportEfforts: ['low', 'high'] }))).toBe('high'); | ||
| }); | ||
|
|
||
| it('returns on for boolean thinking models', () => { | ||
| expect(defaultThinkingLevelFor(model({ capabilities: ['thinking'] }))).toBe('on'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('segmentsFor', () => { | ||
| it('shows off/on for boolean toggle models', () => { | ||
| expect(segmentsFor(model({ capabilities: ['thinking'] }))).toEqual(['on', 'off']); | ||
| }); | ||
|
|
||
| it('shows only on for always-on models', () => { | ||
| expect(segmentsFor(model({ capabilities: ['always_thinking'] }))).toEqual(['on']); | ||
| }); | ||
|
|
||
| it('shows only off for unsupported models', () => { | ||
| expect(segmentsFor(model({ capabilities: [] }))).toEqual(['off']); | ||
| }); | ||
|
|
||
| it('prefixes off to effort lists for toggle effort models', () => { | ||
| expect(segmentsFor(model({ capabilities: ['thinking'], supportEfforts: ['low', 'high', 'max'] }))).toEqual(['off', 'low', 'high', 'max']); | ||
| }); | ||
|
|
||
| it('omits off for always-on effort models', () => { | ||
| expect(segmentsFor(model({ capabilities: ['always_thinking'], supportEfforts: ['low', 'high'] }))).toEqual(['low', 'high']); | ||
| }); | ||
| }); | ||
|
|
||
| describe('coerceThinkingForModel', () => { | ||
| it('keeps the requested level before models are loaded', () => { | ||
| expect(coerceThinkingForModel(undefined, 'high')).toBe('high'); | ||
| }); | ||
|
|
||
| it('forces unsupported models to off', () => { | ||
| expect(coerceThinkingForModel(model({ capabilities: [] }), 'on')).toBe('off'); | ||
| }); | ||
|
|
||
| it('forces always-on models to their default level', () => { | ||
| expect(coerceThinkingForModel(model({ capabilities: ['always_thinking'] }), 'off')).toBe('on'); | ||
| }); | ||
|
|
||
| it('keeps off for boolean toggle models', () => { | ||
| expect(coerceThinkingForModel(model({ capabilities: ['thinking'] }), 'off')).toBe('off'); | ||
| }); | ||
|
|
||
| it('normalizes non-off levels to on for boolean toggle models', () => { | ||
| expect(coerceThinkingForModel(model({ capabilities: ['thinking'] }), 'high')).toBe('on'); | ||
| }); | ||
|
|
||
| it('keeps declared effort levels', () => { | ||
| expect(coerceThinkingForModel(model({ capabilities: ['thinking'], supportEfforts: ['low', 'high', 'max'] }), 'high')).toBe('high'); | ||
| }); | ||
|
|
||
| it('falls back to default effort for undeclared effort levels', () => { | ||
| expect(coerceThinkingForModel(model({ capabilities: ['thinking'], supportEfforts: ['low', 'high', 'max'] }), 'medium')).toBe('high'); | ||
| }); | ||
|
|
||
| it('keeps off for effort models by default', () => { | ||
| expect(coerceThinkingForModel(model({ capabilities: ['thinking'], supportEfforts: ['low', 'high', 'max'] }), 'off')).toBe('off'); | ||
| }); | ||
| }); | ||
|
|
||
| const effortModel = model({ capabilities: ['thinking'], supportEfforts: ['low', 'high', 'max'], defaultEffort: 'high' }); | ||
| const booleanModel = model({ capabilities: ['thinking'] }); | ||
| const alwaysOnModel = model({ capabilities: ['always_thinking'] }); | ||
| const unsupportedModel = model({ capabilities: [] }); | ||
|
|
||
| describe('thinkingLevelForModelSwitch', () => { | ||
|
|
||
| it('auto-enables default effort when switching onto an effort model from off', () => { | ||
| expect(thinkingLevelForModelSwitch(effortModel, 'off', true)).toBe('high'); | ||
| }); | ||
|
|
||
| it('keeps off when re-selecting the current effort model', () => { | ||
| expect(thinkingLevelForModelSwitch(effortModel, 'off', false)).toBe('off'); | ||
| }); | ||
|
|
||
| it('coerces carried-over levels for effort models during a switch', () => { | ||
| expect(thinkingLevelForModelSwitch(effortModel, 'high', true)).toBe('high'); | ||
| expect(thinkingLevelForModelSwitch(effortModel, 'medium', true)).toBe('high'); | ||
| }); | ||
|
|
||
| it('does not auto-enable for boolean models', () => { | ||
| expect(thinkingLevelForModelSwitch(booleanModel, 'off', true)).toBe('off'); | ||
| }); | ||
|
|
||
| it('still coerces boolean models to on when carried level is non-off', () => { | ||
| expect(thinkingLevelForModelSwitch(booleanModel, 'high', true)).toBe('on'); | ||
| }); | ||
|
|
||
| it('forces always-on models on even during re-selection', () => { | ||
| expect(thinkingLevelForModelSwitch(alwaysOnModel, 'off', false)).toBe('on'); | ||
| }); | ||
|
|
||
| it('forces unsupported models off during a switch', () => { | ||
| expect(thinkingLevelForModelSwitch(unsupportedModel, 'high', true)).toBe('off'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('effortLabel', () => { | ||
| it('capitalizes effort names', () => { | ||
| expect(effortLabel('off')).toBe('Off'); | ||
| expect(effortLabel('high')).toBe('High'); | ||
| expect(effortLabel('max')).toBe('Max'); | ||
| }); | ||
|
|
||
| it('returns empty string as-is', () => { | ||
| expect(effortLabel('')).toBe(''); | ||
| }); | ||
| }); | ||
|
|
||
| describe('isThinkingOn', () => { | ||
| it('returns false for off only', () => { | ||
| expect(isThinkingOn('off')).toBe(false); | ||
| expect(isThinkingOn('on')).toBe(true); | ||
| expect(isThinkingOn('high')).toBe(true); | ||
| }); | ||
| }); | ||
|
|
||
| describe('commitLevel', () => { | ||
| it('keeps off', () => { | ||
| expect(commitLevel(effortModel, 'off')).toBe('off'); | ||
| }); | ||
|
|
||
| it('resolves on to the model default', () => { | ||
| expect(commitLevel(effortModel, 'on')).toBe('high'); | ||
| }); | ||
|
|
||
| it('passes concrete efforts through', () => { | ||
| expect(commitLevel(effortModel, 'max')).toBe('max'); | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When there is no active session (the new-session/onboarding composer),
prevModelis alwaysundefinedeven though the current picker value comes fromdraftModelorrawState.defaultModel. If the user has thinking set tooffand clicks the already-current effort-capable model in the full picker,isSwitchbecomes true and the next line auto-enables the model's default effort, so simply re-selecting the current draft/default model silently changes the thinking level for the first prompt instead of preservingoff.Useful? React with 👍 / 👎.