-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: replace the secondary-model experiment with a declarative subagent model pool #2700
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
Changes from all commits
454d4c7
fad5ca8
d215e94
dc1eb7c
a89601c
98a4a6d
c620153
dc5f510
b581ec9
6c98b73
c43c69f
add3738
b38912a
a80b9a6
4cf4621
8aeb6ed
dee665f
be160c8
7240d1a
310b470
dc320ab
7f94519
cda3cc1
e16ca0e
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-sdk": minor | ||
| --- | ||
|
|
||
| Remove the secondary-model session API `Session.applyPersistedSecondaryModel`; subagent model selection is configured via `[secondary_model]` in config.toml instead. The `SECONDARY_DERIVED_MODEL_ALIAS` export stays (the v1 engine still synthesizes the entry at runtime, so hosts keep filtering it out of model pickers), and the SDK now also exports `PRIMARY_SUBAGENT_MODEL_CHOICE`, the v2 subagent model pool's reserved `primary` key. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@moonshot-ai/kimi-code": minor | ||
| --- | ||
|
|
||
| Add a configurable model pool for spawned subagents behind the `secondary-model` experiment (`KIMI_CODE_EXPERIMENTAL_SECONDARY_MODEL=1`, or the master flag): with the experiment on, the `/secondary-model` command or the `[secondary_model]` section in config.toml sets a default model or a small named pool that the main agent picks from per spawn. A lone legacy `model` key in the same section keeps working as the fallback default. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,8 @@ | ||
| import { | ||
| effectiveModelAlias, | ||
| PRIMARY_SUBAGENT_MODEL_CHOICE, | ||
| SECONDARY_DERIVED_MODEL_ALIAS, | ||
| type ExperimentalFeatureState, | ||
| type KimiConfig, | ||
| type ModelAlias, | ||
| type PermissionMode, | ||
| type Session, | ||
|
|
@@ -269,6 +269,15 @@ export async function handleSecondaryModelCommand(host: SlashCommandHost, args: | |
| const alias = args.trim(); | ||
| await refreshModelsForPicker(host); | ||
| const models = pickerModelsForHost(host); | ||
| // The pool reserves `primary` as the symbolic "caller's own model" choice — | ||
| // a user alias with that name can never be the subagent default. | ||
| delete models[PRIMARY_SUBAGENT_MODEL_CHOICE]; | ||
| if (alias === PRIMARY_SUBAGENT_MODEL_CHOICE) { | ||
| host.showError( | ||
| `"${PRIMARY_SUBAGENT_MODEL_CHOICE}" is reserved by the subagent model pool (it always binds the caller's own model) — rename the [models] alias to use it here.`, | ||
| ); | ||
| return; | ||
| } | ||
| if (Object.keys(models).length === 0) { | ||
| host.showNotice( | ||
| 'No models configured', | ||
|
|
@@ -281,7 +290,10 @@ export async function handleSecondaryModelCommand(host: SlashCommandHost, args: | |
| return; | ||
| } | ||
| const secondary = (await host.harness.getConfig()).secondaryModel; | ||
| showSecondaryModelPicker(host, models, secondary?.model ?? '', secondary?.defaultEffort, alias); | ||
| // The v2 engine honors a lone legacy `model` key as the fallback pool | ||
| // default — reflect it as the picker's current value. | ||
| const current = secondary?.defaultModel ?? secondary?.model ?? ''; | ||
| showSecondaryModelPicker(host, models, current, alias.length > 0 ? alias : undefined); | ||
| } | ||
|
|
||
| export async function handleEffortCommand(host: SlashCommandHost, args: string): Promise<void> { | ||
|
|
@@ -427,8 +439,8 @@ async function applyEditorChoice(host: SlashCommandHost, value: string): Promise | |
| /** | ||
| * The models a picker may offer: the user's configured aliases with | ||
| * host-effective provider resolution applied, minus the synthesized | ||
| * `__secondary__` derived entry — a runtime artifact of the `[secondary_model]` | ||
| * recipe that must never be selectable as a primary or secondary model. | ||
| * `__secondary__` derived entry — a runtime artifact of the v1 engine's | ||
| * `[secondary_model]` recipe that must never be selectable as a model. | ||
| */ | ||
| function pickerModelsForHost(host: SlashCommandHost): Record<string, ModelAlias> { | ||
| return Object.fromEntries( | ||
|
|
@@ -604,26 +616,28 @@ async function persistModelSelection( | |
| } | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // Secondary model (`/secondary_model`) | ||
| // Secondary model (`/secondary-model`) — persists `[secondary_model] default_model` | ||
| // --------------------------------------------------------------------------- | ||
|
|
||
| function showSecondaryModelPicker( | ||
| host: SlashCommandHost, | ||
| models: Record<string, ModelAlias>, | ||
| currentValue: string, | ||
| currentEffort: string | undefined, | ||
| selectedValue?: string, | ||
| ): void { | ||
| host.mountEditorReplacement( | ||
| new TabbedModelSelectorComponent({ | ||
| models, | ||
| currentValue, | ||
| selectedValue, | ||
| currentThinkingEffort: currentEffort ?? 'off', | ||
| currentThinkingEffort: 'off', | ||
| // Subagent pool bindings carry no explicit thinking level, so the picker | ||
| // hides the Thinking footer instead of offering a no-op choice. | ||
| thinkingControl: false, | ||
| title: ' Select a secondary model (subagents)', | ||
| onSelect: ({ alias, thinking }) => { | ||
| onSelect: ({ alias }) => { | ||
| host.restoreEditor(); | ||
| void performSecondaryModelSwitch(host, alias, thinking); | ||
| void performSecondaryModelSave(host, alias); | ||
| }, | ||
| onCancel: () => { | ||
| host.restoreEditor(); | ||
|
|
@@ -633,65 +647,32 @@ function showSecondaryModelPicker( | |
| } | ||
|
|
||
| /** | ||
| * Persist-first, then live-apply: the synthesized derived entry only exists in | ||
| * the core config after a reload. No session-only variant — a session-local | ||
| * recipe with patch fields would bind a derived alias the core config cannot | ||
| * resolve. | ||
| * Persists `[secondary_model] default_model`. When a | ||
| * `[secondary_model.models]` pool exists and does not list the alias yet, the | ||
| * alias is added with an empty description — the engine requires the default | ||
| * to be a pool key. Without a pool the default alone forms an implicit | ||
| * single-entry pool, so nothing else is written. No live-apply step: the | ||
| * engine resolves the pool per spawn, so the next subagent dispatch picks the | ||
| * new value up on its own. | ||
| */ | ||
| async function performSecondaryModelSwitch( | ||
| host: SlashCommandHost, | ||
| alias: string, | ||
| effort: ThinkingEffort, | ||
| ): Promise<void> { | ||
| async function performSecondaryModelSave(host: SlashCommandHost, alias: string): Promise<void> { | ||
| const displayName = modelDisplayName(alias, host.state.appState.availableModels[alias]); | ||
| let updatedConfig: KimiConfig; | ||
| try { | ||
| updatedConfig = await host.harness.setConfig({ | ||
| secondaryModel: { model: alias, defaultEffort: effort }, | ||
| }); | ||
| const config = await host.harness.getConfig({ reload: true }); | ||
| const existing = config.secondaryModel?.models; | ||
| const patch: { defaultModel: string; models?: Record<string, string> } = { | ||
| defaultModel: alias, | ||
|
Comment on lines
+663
to
+664
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.
When the CLI is started with Useful? React with 👍 / 👎. |
||
| }; | ||
| if (existing !== undefined) { | ||
| patch.models = { ...existing, [alias]: existing[alias] ?? '' }; | ||
| } | ||
| await host.harness.setConfig({ secondaryModel: patch }); | ||
| } catch (error) { | ||
| host.showError(`Failed to save secondary model: ${formatErrorMessage(error)}`); | ||
| return; | ||
| } | ||
| if (host.session !== undefined) { | ||
| try { | ||
| await host.session.applyPersistedSecondaryModel(); | ||
| } catch (error) { | ||
| host.showError( | ||
| `Saved ${displayName} as the secondary model, but failed to apply it to this session: ${formatErrorMessage(error)}`, | ||
| ); | ||
| return; | ||
| } | ||
| } | ||
| host.setAppState({ availableModels: updatedConfig.models ?? {} }); | ||
| // Report the effective binding from the reloaded config, not the picked | ||
| // value: KIMI_SECONDARY_MODEL / KIMI_SECONDARY_EFFORT override the recipe at | ||
| // runtime, and the session binds the overlaid snapshot (mirrors how | ||
| // /model displays the effective alias read back from the session). | ||
| const effective = updatedConfig.secondaryModel; | ||
| const envOverrides: string[] = []; | ||
| if (effective?.model !== undefined && effective.model !== alias) { | ||
| envOverrides.push(`KIMI_SECONDARY_MODEL=${effective.model}`); | ||
| } | ||
| if (effective?.defaultEffort !== undefined && effective.defaultEffort !== effort) { | ||
| envOverrides.push(`KIMI_SECONDARY_EFFORT=${effective.defaultEffort}`); | ||
| } | ||
| if (envOverrides.length > 0 && effective?.model !== undefined) { | ||
| const effectiveName = modelDisplayName( | ||
| effective.model, | ||
| updatedConfig.models?.[effective.model], | ||
| ); | ||
| host.showStatus( | ||
| `Saved ${displayName} as the secondary model, but ${envOverrides.join(' and ')} ` + | ||
| `overrides it at runtime — subagents bind ${effectiveName} until the env var is unset.`, | ||
| 'warning', | ||
| ); | ||
| return; | ||
| } | ||
| host.showStatus( | ||
| host.session === undefined | ||
| ? `Secondary model set to ${displayName} with thinking ${effort}; applies to new sessions.` | ||
| : `Secondary model set to ${displayName} with thinking ${effort}.`, | ||
| `Secondary model set to ${displayName}. Newly spawned subagents will use it by default.`, | ||
| 'success', | ||
| ); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -185,8 +185,8 @@ export const BUILTIN_SLASH_COMMANDS = [ | |
| availability: 'always', | ||
| }, | ||
| { | ||
| name: 'secondary_model', | ||
| aliases: [], | ||
| name: 'secondary-model', | ||
| aliases: ['subagent-model'], | ||
|
Comment on lines
+188
to
+189
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.
With the AGENTS.md reference: AGENTS.md:L59-L61 Useful? React with 👍 / 👎. |
||
| description: 'Configure the secondary model for subagents', | ||
| priority: 90, | ||
| availability: 'always', | ||
|
|
||
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.
The changeset declares a minor SDK bump while the same commit deletes the public
Session.applyPersistedSecondaryModel()method and its RPC surface. Existing JavaScript consumers will receiveundefinedat runtime, and TypeScript consumers can no longer compile after an otherwise compatible-range upgrade; either retain a deprecated compatibility shim or release the removal as a major change.AGENTS.md reference: AGENTS.md:L59-L61
Useful? React with 👍 / 👎.