v1.5.0: dynamic model list — server-driven, not hardcoded. - #10
Open
boogy777-lgtm wants to merge 2 commits into
Open
v1.5.0: dynamic model list — server-driven, not hardcoded.#10boogy777-lgtm wants to merge 2 commits into
boogy777-lgtm wants to merge 2 commits into
Conversation
Mirror kimi-cli refresh_managed_models: GET /coding/v1/models is now the
single source of truth for the model family and metadata. A new config hook
injects the provider entry + one model entry per discovered model at every
opencode startup (best-effort, static fallback on failure), so newly
released Kimi Code models appear in the picker without a plugin update or
manual config block — same UX as opencode-gemini-auth/antigravity-auth.
• config hook: startup model sync plus provider entry injection
• provider.models: materializes discovered models missing from config
• isKimiModel gates on discovered ids ∪ static fallback, not hardcoded list
• buildConfigBlock generated from discovery (context_length server-reported)
• FALLBACK_MODEL_IDS/FALLBACK_MODELS constants — cold-start only
• MAX_REASONING_MODEL_IDS stays explicit (capability gate, not model list)
• deep per-field merge: user config overrides win, never deleted
• 6 new tests, 85/85 green, tsc clean
📝 WalkthroughWalkthroughThe plugin now discovers multiple Kimi models, generates fallback metadata, synchronizes provider configuration at startup, preserves user overrides, and applies model-specific limits and reasoning variants. ChangesKimi model discovery and configuration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PluginConfigHook
participant ModelDiscovery
participant ProviderConfig
PluginConfigHook->>ModelDiscovery: discover models before startup loading
ModelDiscovery-->>PluginConfigHook: discovered or fallback model list
PluginConfigHook->>ProviderConfig: merge overrides and upsert provider models
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/constants.ts (1)
35-61: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider deriving
FALLBACK_MODEL_IDSfromFALLBACK_MODELS.The two constants list the same three ids. A future edit to one list can leave the other stale, which silently changes the cold-start known-model gate.
♻️ Proposed refactor
-export const FALLBACK_MODEL_IDS = ["k3", "kimi-for-coding", "kimi-for-coding-highspeed"] as const - -// Fallback per-model metadata for the same cold-start path. Server-reported +// Fallback per-model metadata for the cold-start path. Server-reported // `display_name` / `context_length` / `supports_*` flags always win over these. export const FALLBACK_MODELS: ReadonlyArray<{ id: string display_name: string context_length: number supports_image_in: boolean supports_video_in: boolean }> = [Then declare the id list after the table:
export const FALLBACK_MODEL_IDS: ReadonlyArray<string> = FALLBACK_MODELS.map((m) => m.id)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/constants.ts` around lines 35 - 61, Derive FALLBACK_MODEL_IDS from FALLBACK_MODELS instead of maintaining a duplicate literal list. Move its declaration after the FALLBACK_MODELS table and map each model’s id, preserving the existing exported read-only string-array contract.test/plugin.test.ts (1)
227-258: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding a clamp test for a model outside
MAX_REASONING_MODEL_IDSthat is known only from discovery.The new tests cover
k3andkimi-for-coding. The documented rule also clampsmaxtohighfor any newly discovered model until it is verified. A test for a discovered id such ask4withreasoning_effort: "max"would lock that safe default in.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/plugin.test.ts` around lines 227 - 258, Add a chat.params test for a discovered model ID outside MAX_REASONING_MODEL_IDS, such as k4, using reasoning_effort "max"; assert the output clamps reasoning_effort to "high" and enables thinking, matching the existing kimi-for-coding coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/index.ts`:
- Around line 748-761: Restrict the legacy MODEL_ID fallback in the discovered
model lookup to cases where auth.models contains exactly one model. When
discovery has zero or multiple models and does not contain MODEL_ID, leave
requestedModelId unchanged so the server handles the entitlement error; preserve
direct matches and the existing single-model fallback behavior.
---
Nitpick comments:
In `@src/constants.ts`:
- Around line 35-61: Derive FALLBACK_MODEL_IDS from FALLBACK_MODELS instead of
maintaining a duplicate literal list. Move its declaration after the
FALLBACK_MODELS table and map each model’s id, preserving the existing exported
read-only string-array contract.
In `@test/plugin.test.ts`:
- Around line 227-258: Add a chat.params test for a discovered model ID outside
MAX_REASONING_MODEL_IDS, such as k4, using reasoning_effort "max"; assert the
output clamps reasoning_effort to "high" and enables thinking, matching the
existing kimi-for-coding coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: eaad6917-2ff8-489a-87d5-f190408c31ca
📒 Files selected for processing (6)
AGENTS.mdREADME.mdpackage.jsonsrc/constants.tssrc/index.tstest/plugin.test.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/constants.ts">
<violation number="1" location="src/constants.ts:35">
P2: After `/coding/v1/models` succeeds, a model omitted by the account can still be treated as a Kimi model because this fallback ID remains in the permanent gate. Restrict the post-discovery gate to discovered IDs and use the fallback IDs only while no discovery list exists, so removed or unauthorized models are not sent through Kimi-specific request handling.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // model list at all. The entries below are only the cold-start fallback used | ||
| // when discovery has not produced a list yet in this process (offline start, | ||
| // not logged in, or the chat hooks firing before the first discovery). | ||
| export const FALLBACK_MODEL_IDS = ["k3", "kimi-for-coding", "kimi-for-coding-highspeed"] as const |
There was a problem hiding this comment.
P2: After /coding/v1/models succeeds, a model omitted by the account can still be treated as a Kimi model because this fallback ID remains in the permanent gate. Restrict the post-discovery gate to discovered IDs and use the fallback IDs only while no discovery list exists, so removed or unauthorized models are not sent through Kimi-specific request handling.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/constants.ts, line 35:
<comment>After `/coding/v1/models` succeeds, a model omitted by the account can still be treated as a Kimi model because this fallback ID remains in the permanent gate. Restrict the post-discovery gate to discovered IDs and use the fallback IDs only while no discovery list exists, so removed or unauthorized models are not sent through Kimi-specific request handling.</comment>
<file context>
@@ -22,8 +22,55 @@ export const OAUTH_DEVICE_GRANT = "urn:ietf:params:oauth:grant-type:device_code"
+// model list at all. The entries below are only the cold-start fallback used
+// when discovery has not produced a list yet in this process (offline start,
+// not logged in, or the chat hooks firing before the first discovery).
+export const FALLBACK_MODEL_IDS = ["k3", "kimi-for-coding", "kimi-for-coding-highspeed"] as const
+
+// Fallback per-model metadata for the same cold-start path. Server-reported
</file context>
…ambiguous slug fallback
• config hook refreshes an expiring stored token before startup discovery
(Kimi access tokens live ~15 min — without this, cold starts always
401'd and degraded to the static fallback) + one forced-refresh retry on 401
• chat-hook gate is the discovered id set once discovery succeeded;
fallback-id models the server omits drop out (fail loud server-side)
• rememberDiscovery overwrites even on empty list — a successful response
is authoritative, stale entitlements must not survive it
• MODEL_ID wire-slug fallback only when discovery returned exactly one
model; multi-model discovery leaves the id untouched so the server
answers with a visible entitlement error instead of silently running a
different model
• FALLBACK_MODEL_IDS derived from FALLBACK_MODELS (no duplicate literal)
• tests: max→high clamp for discovered-only model, gate drop-out after
discovery, refresh-before-discovery, 401 retry, fixed dead mock in the
network-down test
• 89/89 green, tsc clean
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/index.ts`:
- Around line 105-113: Update knownModelIds to use the presence of
discovery.models, not its length, so an authoritative empty array returns an
empty set while undefined still uses FALLBACK_MODEL_IDS. Apply the same
undefined-versus-defined check to the config fallback near config and
FALLBACK_MODELS, preserving the existing cachedDiscovery consumers in
chat.headers and chat.params through the corrected knownModelIds behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b951ad4e-59f7-49cd-b5dc-a03c66710731
📒 Files selected for processing (4)
AGENTS.mdsrc/constants.tssrc/index.tstest/plugin.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/constants.ts
Comment on lines
+105
to
+113
| // The known set is the server-discovered list once discovery has succeeded in | ||
| // this process (kimi-cli keeps no static list at all — removed/entitled-out | ||
| // models drop out of the gate, so their requests fail loud server-side | ||
| // instead of carrying Kimi-specific fields). The static fallback only covers | ||
| // the window before the first discovery. | ||
| function knownModelIds(discovery: ModelDiscovery): ReadonlySet<string> { | ||
| if (discovery.models?.length) return new Set(discovery.models.map((m) => m.id)) | ||
| return new Set(FALLBACK_MODEL_IDS) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
knownModelIds cannot distinguish "not yet discovered" from "discovered, zero models".
ModelDiscovery.models is optional, and rememberDiscovery (Line 537) deliberately sets cachedDiscovery = { models } even for an empty array, because a successful /models response is treated as authoritative. However, knownModelIds checks discovery.models?.length (Line 111), which is falsy both when discovery has never run and when discovery legitimately returned zero models. In the latter case, the function still returns FALLBACK_MODEL_IDS (Line 112), contradicting its own comment that "the static fallback only covers the window before the first discovery."
Practical effect: if an account's entitlement drops to zero models, a successful (authoritative) empty response should stop treating any id as Kimi-capable, but this code silently re-enables gating for the stale fallback ids instead. The same .length ambiguity is inherited at:
- Line 612, where
configfalls back toFALLBACK_MODELSfor the same authoritative-empty case. - Line 862 and Line 884, where
chat.headers/chat.paramscallknownModelIds(cachedDiscovery)and therefore inherit the same misclassification.
Distinguish "undefined" from "defined-but-empty" instead of using .length.
🐛 Proposed fix for the discovery-state check
function knownModelIds(discovery: ModelDiscovery): ReadonlySet<string> {
- if (discovery.models?.length) return new Set(discovery.models.map((m) => m.id))
- return new Set(FALLBACK_MODEL_IDS)
+ if (discovery.models) return new Set(discovery.models.map((m) => m.id))
+ return new Set(FALLBACK_MODEL_IDS)
}Also applies to: 612-612, 862-862, 884-884
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/index.ts` around lines 105 - 113, Update knownModelIds to use the
presence of discovery.models, not its length, so an authoritative empty array
returns an empty set while undefined still uses FALLBACK_MODEL_IDS. Apply the
same undefined-versus-defined check to the config fallback near config and
FALLBACK_MODELS, preserving the existing cachedDiscovery consumers in
chat.headers and chat.params through the corrected knownModelIds behavior.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Makes the Kimi Code model list server‑driven. On startup we sync
/coding/v1/models, refresh the token if needed, and inject the provider + discovered models so new Kimi models appear without plugin updates or config edits.New Features
kimi-for-coding-oauthwith one entry per discovered model; falls back to a small static list on failure; user overrides are merged per-field.provider.modelsupdatesdisplay_name,context_length, and image/video support for all known Kimi Code ids and materializes newly discovered models missing from config; Kimi fields (prompt_cache_key,thinking,reasoning_effort) gate on discovered ∪ fallback ids.maxis sent fork3;maxandxhighclamp tohighfor K2.7/unknown models.modelrewrites to the discovered slug only when discovery returned exactly one model; the legacykimi-for-codingalias remains supported.limit: { context, output: 65536 },attachment: true,modalitiesfrom discovery, andvariantsincludingmax.Migration
Written for commit 7e19876. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores