feat(model-picker): surface inactive provider profiles in /model (#1119 piece 2)#1164
feat(model-picker): surface inactive provider profiles in /model (#1119 piece 2)#11640xfandom wants to merge 1 commit into
Conversation
When a user configures multiple providerProfiles (Kimi + Z.AI + OpenRouter + SambaNova in the Gitlawb#1119 repro, but the pattern fits any multi-provider setup), switching the main session between them currently requires round-tripping through /provider — /model only shows the active profile's models. Make /model the single switcher: - ModelOption gains an optional `switchToProfileId`. Existing options leave it unset and behave exactly as today. - `getInactiveProviderProfileOptions` enumerates every configured profile that isn't the active one and emits a picker entry per model, labelled `<model> · <profile.name>` so the user can see the choice changes providers, not just models. - Each option's `value` is encoded with `__switch_profile__:<id>:<model>` so the picker's plain-string `value` channel stays the source of truth and same-named models under different base URLs (`gpt-4o` on multiple OpenAI-compatible endpoints) stay disambiguated. - /model's handleSelect detects the prefix, calls `setActiveProviderProfile` (same path /provider uses — applies env, persists active profile, refreshes startup file), then sets `mainLoopModel` to the bare model string. Only surfaces inactive options when `CLAUDE_CODE_PROVIDER_PROFILE_ENV_APPLIED` is set, so users who haven't opted into the multi-profile workflow at all don't see the affordance. Tests cover round-trip encoding (including OpenRouter-style colon-bearing model strings), the active-filter, the multi-model explosion, and that `getModelOptions()` 3P path includes the inactive options only when the profile env is applied. Combined invocation with the rest of `src/utils/model/` + `src/commands/model/` + `src/utils/providerProfiles.test.ts` runs clean to guard against mock-leak (per the 2026-04-30 lesson — spreads `import * as actual` for every `mock.module` factory). Refs Gitlawb#1119
gnanam1990
left a comment
There was a problem hiding this comment.
Local: bun test src/utils/model/modelOptions.crossProfile.test.ts → 8/0 pass.
Endorsing @kevincodex1. The composite-value design is the right tradeoff — keeps the picker's value channel a plain string, and parseSwitchProfileValue correctly rejects malformed forms (<prefix>::foo, <prefix>:bar:). Gating on CLAUDE_CODE_PROVIDER_PROFILE_ENV_APPLIED so the inactive-profile entries only surface for users who've opted into the multi-profile workflow is a nice touch. setActiveProviderProfile() before setAppState ordering is correct.
Nit (non-blocking): the new logEvent('tengu_model_command_menu', { action: 'switch_profile' }) matches the existing 2 calls in this file, so it's consistent — but the whole tengu_* namespace is fork tech-debt that's worth a separate scrub across the codebase before/after the openlawb rename.
No other red flags. LGTM.
jatmn
left a comment
There was a problem hiding this comment.
Findings
- [P2] Cross-profile
/modelswitches skip the existing fast-mode cleanup
src/commands/model/model.tsx:390
The newparseSwitchProfileValue()branch returns immediately after activating the target profile, so it never reaches the fast-mode reconciliation that the normal/modelpath runs a few lines below. That means a first-party user can turn fast mode on, switch to an inactive provider profile from the picker, and keepfastModelatched even though the picker explicitly says switching away from the fast model turns it off. At best that leaves stale state until the user toggles/fastmanually; at worst it silently re-enables fast mode when they come back to Anthropic later in the session. Please run the same fast-mode cleanup for switch-profile selections and cover that branch with a command-level test.
Blockers
Non-Blocking
Looks Good
Verdict: Changes Requested — fast-mode cleanup needs to run for switch-profile selections. |
|
Re-checked against the code: @jatmn's finding holds, and my earlier approval predates his review and is superseded by it. The new |
Summary
Follow-up to #1146 (which lands the Codex/GPT suppression piece). This PR addresses the second piece @Vasanthdev2004 split out — making
/modela single switcher across configuredproviderProfilesinstead of forcing a/providerround-trip.ModelOptiongains an optionalswitchToProfileId. Existing options leave it unset and behave exactly as today.getInactiveProviderProfileOptions()enumerates every configured profile that isn't the active one and emits a picker entry per model, labelled<model> · <profile.name>so the user can see they're changing providers, not just the model — addresses Vasanthdev's concern about confusing "change model only" with "change provider/base URL/API key".valueis encoded with__switch_profile__:<id>:<model>so the picker's plain-stringvaluechannel stays the source of truth and same-named models under different base URLs (e.g.gpt-4oon multiple OpenAI-compatible endpoints) stay disambiguated. Decoded via the newparseSwitchProfileValuehelper./model'shandleSelectdetects the prefix, callssetActiveProviderProfile(id)(the same call path/provideruses — applies env, persists active profile, refreshes startup file), then setsmainLoopModelto the bare model string. No new env-management surface.CLAUDE_CODE_PROVIDER_PROFILE_ENV_APPLIEDis set, so users who haven't opted into the multi-profile workflow don't see the affordance.With @suenot's 4-profile repro (Kimi active, GLM-5.1 / DeepSeek V4 Flash / MiniMax inactive), the picker now reads:
Impact
/modelbecomes the unified switcher requested in Feature: Show agentModels from settings.json in /model picker instead of hardcoded GPT/Codex models #1119 for multi-profile users. Users with a single profile see no behavior change (no inactive profiles → empty append).ModelOption(optional, no downstream consumer requires it), two small pure helpers (parseSwitchProfileValue/encodeSwitchProfileValue), and one newhandleSelectbranch in the/modelcommand. No change toModelPicker'sonSelect: (model, effort)shape — the profile id is carried through thevaluechannel.Testing
bun run build— greenbun test src/utils/model/modelOptions.crossProfile.test.ts— 8/8 pass (round-trip encode/parse,:preservation for OpenRouter model strings, active-filter, multi-model explosion, env-applied vs not).bun test src/utils/model/ src/commands/model/ src/utils/providerProfiles.test.ts— 165/165 pass.bun run smoke— fails on this checkout withCannot find package '@orama/orama'; reproduces on cleanmainHEAD4d2de51(pre-existing local env issue, unrelated to this change).Notes
getModelOptions()3P path (getAPIProvider() === 'openai', non-subscriber, profile env applied). The 1P path also appendsinactiveProfileOptionsso a 1P user with extra 3P profiles configured can still see them, mirroring howprofileModelOptionsis already appended in both branches.:separator inside the encodedvalueonly splits on the FIRST colon — explicitly tested withdeepseek/deepseek-v4-flash:nitrosince OpenRouter model strings carry:segments.agentModelsfromsettings.json(different shape — those are subagent-routing entries with their ownbase_url/api_key; a separate UX decision about whether to expose subagent providers as first-class/modeloptions).mock.modulesurface leaks: everymock.module(path, factory)call in the new test file spreadsimport * as actual from pathand only overrides the symbols the test needs.Refs #1119 — piece 2 of two.