Skip to content

feat(llm): add Kilo Gateway and OpenCode Go provider support - #225

Merged
jamiepine merged 6 commits into
spacedriveapp:mainfrom
skulldogged:provider-additions
Feb 26, 2026
Merged

feat(llm): add Kilo Gateway and OpenCode Go provider support#225
jamiepine merged 6 commits into
spacedriveapp:mainfrom
skulldogged:provider-additions

Conversation

@skulldogged

@skulldogged skulldogged commented Feb 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds first-class support for Kilo Gateway and OpenCode Go, expanding Spacebot to 13 supported LLM providers. Also refactors provider configuration to reduce duplication.

Changes

Kilo Gateway:

  • Added kilo_key and KILO_API_KEY support with automatic provider registration
  • New ApiType::KiloGateway with /chat/completions endpoint and required headers (HTTP-Referer, X-Title)
  • Routing defaults: Claude Sonnet 4.5 for channels/branches, Claude Haiku 4.5 for workers
  • Custom Kilo icon in Settings UI

OpenCode Go:

  • Added opencode_go_key and OPENCODE_GO_API_KEY support
  • Routing defaults: kimi-k2.5 for channels/branches, kimi-k2 for workers
  • Reuses OpenCode Zen icon in Settings UI

Refactoring:

  • Extracted provider configs into default_provider_config() to eliminate duplication between production code and API tests
  • Added models.dev provider mappings: opencodeopencode-zen, opencode-go, zai-coding-plan, minimax, moonshotaimoonshot
  • Removed 15 duplicate models from extra_models() that are now fetched from models.dev

Documentation:

  • Updated README, config guide, quickstart, and roadmap (13 providers total)

Why

Kilo Gateway offers an OpenAI-compatible multi-provider gateway with a unique endpoint structure, while OpenCode Go provides access to kimi-k2.5 and other models via a separate inference tier. Both providers work out of the box with proper routing and no additional configuration beyond API keys.

Supersedes #189 and #224.


Note

This PR introduces two new LLM providers with centralized configuration support. The refactoring extracts duplicate provider setup logic into a single function used by both production code and tests, improving maintainability. The changes span configuration handling, provider initialization, routing defaults, documentation updates, and model mappings through models.dev integration. Spans 14 files with 534 additions and 332 deletions.

Written by Tembo for commit b9bda6d.

@coderabbitai

coderabbitai Bot commented Feb 26, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds two LLM providers (Kilo Gateway and OpenCode Go) across docs, frontend, config, routing, and runtime. Introduces new LLM keys (kilo_key, opencode_go_key), ApiType variants, provider defaults, routing entries, and per-call header support for Kilo Gateway.

Changes

Cohort / File(s) Summary
Documentation
README.md, docs/content/docs/(configuration)/config.mdx, docs/content/docs/(deployment)/roadmap.mdx, docs/content/docs/(getting-started)/quickstart.mdx
Add Kilo Gateway and OpenCode Go to provider lists/examples, document new kilo_key / opencode_go_key, update fallback/provider examples and base-URL/endpoint notes.
Frontend UI & API types
interface/src/api/client.ts, interface/src/components/ModelSelect.tsx, interface/src/lib/providerIcons.tsx, interface/src/routes/Settings.tsx
Add kilo/opencode_go flags to provider status, new provider labels/order and providerRank sorting, add Kilo icon and opencode-go alias, and Settings entries (placeholders, envVar, defaultModel).
Config core
src/config.rs
Add ApiType variants (OpenAiChatCompletions, KiloGateway), add kilo_key/opencode_go_key to TOML/LlmConfig, new base-URL constants, legacy env fallbacks, shorthand provider wiring, and default_provider_config helper; update tests.
LLM runtime & model routing
src/llm/model.rs, src/llm/providers.rs, src/llm/routing.rs
Switch OpenAI-compatible routing to ApiType-based matching; add extra_headers support and include Kilo-specific headers; log provider-configured messages for Kilo/OpenCode Go; add routing defaults/prefixes for kilo and opencode-go.
API / models layer
src/api/models.rs, src/api/providers.rs
Expand direct provider mappings and adjust model catalog; add kilo/opencode_go fields to ProviderStatus; map TOML keys for kilo/opencode-go; normalize provider inputs and include new keys in config population and get_providers.
Misc / tests
*
Various tests, onboarding text, examples, and startup wiring updated to include new providers and env keys.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding support for two new LLM providers (Kilo Gateway and OpenCode Go) with appropriate context (feat/llm scope).
Description check ✅ Passed The description comprehensively explains the changes across both new providers, refactoring efforts, documentation updates, and spans across all modified files with clear context about objectives and rationale.
Docstring Coverage ✅ Passed Docstring coverage is 94.44% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/config.rs (1)

4556-4579: ⚠️ Potential issue | 🔴 Critical

Duplicate match arm index causes unreachable pattern.

The match arm at line 4578 uses index 16 which is already used by Z.AI Coding Plan at line 4573. Kilo Gateway is at index 17 in the providers array but the match uses 16.

🐛 Proposed fix
         16 => (
             "Z.AI Coding Plan API key",
             "zai_coding_plan_key",
             "zai-coding-plan",
         ),
-        16 => ("Kilo Gateway API key", "kilo_key", "kilo"),
+        17 => ("Kilo Gateway API key", "kilo_key", "kilo"),
         _ => unreachable!(),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/config.rs` around lines 4556 - 4579, The match on provider_idx that
assigns (provider_input_name, toml_key, provider_id) has a duplicate arm for 16
(used twice for "Z.AI Coding Plan" and "Kilo Gateway"), causing the later arm to
be unreachable; update the second duplicate arm to use 17 (matching the
providers array index) so provider_idx maps correctly to "Kilo Gateway" for
provider_input_name/toml_key/provider_id in the match block that sets these
variables.
🧹 Nitpick comments (1)
src/config.rs (1)

349-493: Remove redundant use statement and consider removing unnecessary clones.

Line 355 has use crate::config::ApiType; which is redundant since ApiType is already in scope within this module. Additionally, the api_key.clone() calls throughout the match arms are unnecessary since api_key is an owned String that's not used after the match.

♻️ Proposed simplification
 pub(crate) fn default_provider_config(
     provider_id: &str,
     api_key: impl Into<String>,
 ) -> Option<ProviderConfig> {
-    use crate::config::ApiType;
     let api_key = api_key.into();
     Some(match provider_id {
         "anthropic" => ProviderConfig {
             api_type: ApiType::Anthropic,
             base_url: ANTHROPIC_PROVIDER_BASE_URL.to_string(),
-            api_key: api_key.clone(),
+            api_key,
             name: None,
             use_bearer_auth: false,
         },
         // ... similar changes for other arms, using `api_key` directly in the last arm

Note: If only one arm can match, you can use api_key directly in that arm. Since all arms consume the value, you'd need to restructure slightly or keep the clones for flexibility.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/config.rs` around lines 349 - 493, Remove the redundant `use
crate::config::ApiType;` and stop cloning the API key repeatedly: change the
function signature of default_provider_config to accept api_key: String (instead
of impl Into<String>), remove the let api_key = api_key.into(); and restructure
the match so the owned api_key is moved into the single matching arm (use
api_key directly in each ProviderConfig construction, not api_key.clone());
reference symbols: default_provider_config, ProviderConfig, and the match arms
like "anthropic", "openai", etc.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/content/docs/`(configuration)/config.mdx:
- Line 178: The docs text listing implicit environment fallbacks is missing
OPENCODE_GO_API_KEY; update the sentence that enumerates implicit env fallbacks
to include OPENCODE_GO_API_KEY so it matches the implementation (see
opencode_go_key in providers.rs which falls back to OPENCODE_GO_API_KEY).

In `@docs/content/docs/`(getting-started)/quickstart.mdx:
- Around line 50-51: Update the quickstart list entry that currently reads "**An
LLM API key** — Anthropic, OpenAI, OpenRouter, or Kilo Gateway" to include
OpenCode Go (e.g., add "OpenCode Go" alongside the other providers), and make
the same addition where the providers are referenced later (around lines
referenced in the comment, originally 89-93); locate the markdown block by
searching for the exact phrase "**An LLM API key** — Anthropic, OpenAI,
OpenRouter, or Kilo Gateway" and update the text to mention OpenCode Go so the
quickstart reflects the new first-class provider.

In `@README.md`:
- Line 195: Update the README provider list to also mention "OpenCode Go"
alongside Kilo Gateway, NVIDIA, MiniMax, Moonshot AI (Kimi), and Z.AI Coding
Plan and include its corresponding configuration key under the [llm] section
(use the project naming convention, e.g., opencode_go_key) so users know how to
configure it; edit the same README.md line that currently lists "Kilo Gateway,
NVIDIA, MiniMax, Moonshot AI (Kimi), and Z.AI Coding Plan" and add "OpenCode Go
— configure with opencode_go_key" to match the existing pattern of "kilo_key,
nvidia_key, minimax_key, moonshot_key, zai_coding_plan_key".

In `@src/config.rs`:
- Line 4791: The KEYS constant declaration in src/config.rs lists 24 string
entries but is declared as const KEYS: [&str; 23], causing a compile-time array
size mismatch; update the declaration to match the actual number of elements
(e.g., change the array length from 23 to 24) or remove the explicit length and
use a slice/Vec so that the added keys KILO_API_KEY and OPENCODE_GO_API_KEY are
included without causing a type mismatch.

---

Outside diff comments:
In `@src/config.rs`:
- Around line 4556-4579: The match on provider_idx that assigns
(provider_input_name, toml_key, provider_id) has a duplicate arm for 16 (used
twice for "Z.AI Coding Plan" and "Kilo Gateway"), causing the later arm to be
unreachable; update the second duplicate arm to use 17 (matching the providers
array index) so provider_idx maps correctly to "Kilo Gateway" for
provider_input_name/toml_key/provider_id in the match block that sets these
variables.

---

Nitpick comments:
In `@src/config.rs`:
- Around line 349-493: Remove the redundant `use crate::config::ApiType;` and
stop cloning the API key repeatedly: change the function signature of
default_provider_config to accept api_key: String (instead of impl
Into<String>), remove the let api_key = api_key.into(); and restructure the
match so the owned api_key is moved into the single matching arm (use api_key
directly in each ProviderConfig construction, not api_key.clone()); reference
symbols: default_provider_config, ProviderConfig, and the match arms like
"anthropic", "openai", etc.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8582124 and b9bda6d.

📒 Files selected for processing (14)
  • README.md
  • docs/content/docs/(configuration)/config.mdx
  • docs/content/docs/(deployment)/roadmap.mdx
  • docs/content/docs/(getting-started)/quickstart.mdx
  • interface/src/api/client.ts
  • interface/src/components/ModelSelect.tsx
  • interface/src/lib/providerIcons.tsx
  • interface/src/routes/Settings.tsx
  • src/api/models.rs
  • src/api/providers.rs
  • src/config.rs
  • src/llm/model.rs
  • src/llm/providers.rs
  • src/llm/routing.rs

Comment thread docs/content/docs/(configuration)/config.mdx Outdated
Comment thread docs/content/docs/(getting-started)/quickstart.mdx Outdated
Comment thread README.md Outdated
Comment thread src/config.rs Outdated
Comment thread src/config.rs Outdated
Comment thread src/llm/model.rs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/config.rs (2)

4791-4816: ⚠️ Potential issue | 🟡 Minor

EnvGuard still omits MINIMAX_CN_API_KEY

This lets host env values leak into tests and can make provider-related tests non-deterministic.

🧪 Proposed fix
-            const KEYS: [&str; 24] = [
+            const KEYS: [&str; 25] = [
                 "SPACEBOT_DIR",
                 "SPACEBOT_DEPLOYMENT",
                 "SPACEBOT_CRON_TIMEZONE",
                 "ANTHROPIC_API_KEY",
                 "ANTHROPIC_OAUTH_TOKEN",
                 "OPENAI_API_KEY",
                 "OPENROUTER_API_KEY",
                 "KILO_API_KEY",
                 "ZHIPU_API_KEY",
                 "GROQ_API_KEY",
                 "TOGETHER_API_KEY",
                 "FIREWORKS_API_KEY",
                 "DEEPSEEK_API_KEY",
                 "XAI_API_KEY",
                 "MISTRAL_API_KEY",
                 "GEMINI_API_KEY",
                 "NVIDIA_API_KEY",
                 "OLLAMA_API_KEY",
                 "OLLAMA_BASE_URL",
                 "OPENCODE_ZEN_API_KEY",
                 "OPENCODE_GO_API_KEY",
                 "MINIMAX_API_KEY",
+                "MINIMAX_CN_API_KEY",
                 "MOONSHOT_API_KEY",
                 "ZAI_CODING_PLAN_API_KEY",
             ];
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/config.rs` around lines 4791 - 4816, The EnvGuard's KEYS constant omits
"MINIMAX_CN_API_KEY", allowing host env values to leak into tests; add
"MINIMAX_CN_API_KEY" to the KEYS array defined in src/config.rs so EnvGuard will
strip that variable during test runs (update the const KEYS list where other API
key names like "MINIMAX_API_KEY" and "MOONSHOT_API_KEY" are declared).

2399-2440: ⚠️ Potential issue | 🟠 Major

needs_onboarding() misses MINIMAX_CN_API_KEY

If only MINIMAX_CN_API_KEY is set, onboarding is incorrectly required even though load_from_env() supports this provider key.

🐛 Proposed fix
         let has_legacy_keys = std::env::var("ANTHROPIC_API_KEY").is_ok()
             || std::env::var("OPENAI_API_KEY").is_ok()
             || std::env::var("OPENROUTER_API_KEY").is_ok()
             || std::env::var("KILO_API_KEY").is_ok()
             || std::env::var("ZHIPU_API_KEY").is_ok()
             || std::env::var("GROQ_API_KEY").is_ok()
             || std::env::var("TOGETHER_API_KEY").is_ok()
             || std::env::var("FIREWORKS_API_KEY").is_ok()
             || std::env::var("DEEPSEEK_API_KEY").is_ok()
             || std::env::var("XAI_API_KEY").is_ok()
             || std::env::var("MISTRAL_API_KEY").is_ok()
             || std::env::var("NVIDIA_API_KEY").is_ok()
             || std::env::var("OLLAMA_API_KEY").is_ok()
             || std::env::var("OLLAMA_BASE_URL").is_ok()
             || std::env::var("OPENCODE_ZEN_API_KEY").is_ok()
             || std::env::var("OPENCODE_GO_API_KEY").is_ok()
             || std::env::var("MINIMAX_API_KEY").is_ok()
+            || std::env::var("MINIMAX_CN_API_KEY").is_ok()
             || std::env::var("MOONSHOT_API_KEY").is_ok()
             || std::env::var("ZAI_CODING_PLAN_API_KEY").is_ok();
@@
         let has_legacy_bootstrap_vars = std::env::var("ANTHROPIC_API_KEY").is_ok()
             || std::env::var("ANTHROPIC_OAUTH_TOKEN").is_ok()
             || std::env::var("OPENAI_API_KEY").is_ok()
             || std::env::var("OPENROUTER_API_KEY").is_ok()
             || std::env::var("KILO_API_KEY").is_ok()
             || std::env::var("OPENCODE_ZEN_API_KEY").is_ok()
-            || std::env::var("OPENCODE_GO_API_KEY").is_ok();
+            || std::env::var("OPENCODE_GO_API_KEY").is_ok()
+            || std::env::var("MINIMAX_CN_API_KEY").is_ok();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/config.rs` around lines 2399 - 2440, The needs_onboarding() logic omits
checking for MINIMAX_CN_API_KEY so if only that env var is present onboarding is
still required; update the has_legacy_keys and has_legacy_bootstrap_vars checks
inside needs_onboarding() to include std::env::var("MINIMAX_CN_API_KEY").is_ok()
(add the same check where legacy keys and bootstrap vars are enumerated) so the
function recognizes the MINIMAX_CN_API_KEY as a valid existing provider key
supported by load_from_env().
🧹 Nitpick comments (3)
src/api/providers.rs (1)

785-829: Normalize provider IDs consistently across provider endpoints.

/providers/test now normalizes provider IDs, but /providers update/delete still operate on raw provider strings. That can make mixed-case inputs pass test and fail save/remove.

♻️ Proposed consistency patch
 pub(super) async fn update_provider(
     State(state): State<Arc<ApiState>>,
     Json(request): Json<ProviderUpdateRequest>,
 ) -> Result<Json<ProviderUpdateResponse>, StatusCode> {
-    let Some(key_name) = provider_toml_key(&request.provider) else {
+    let normalized_provider = request.provider.trim().to_lowercase();
+    let normalized_model = request.model.trim();
+    let Some(key_name) = provider_toml_key(&normalized_provider) else {
         return Ok(Json(ProviderUpdateResponse {
             success: false,
             message: format!("Unknown provider: {}", request.provider),
         }));
     };
@@
-    if !model_matches_provider(&request.provider, &request.model) {
+    if !model_matches_provider(&normalized_provider, normalized_model) {
         return Ok(Json(ProviderUpdateResponse {
             success: false,
             message: format!(
                 "Model '{}' does not match provider '{}'.",
                 request.model, request.provider
             ),
         }));
     }
@@
-    apply_model_routing(&mut doc, request.model.as_str());
+    apply_model_routing(&mut doc, normalized_model);
 pub(super) async fn delete_provider(
     State(state): State<Arc<ApiState>>,
     axum::extract::Path(provider): axum::extract::Path<String>,
 ) -> Result<Json<ProviderUpdateResponse>, StatusCode> {
+    let provider = provider.trim().to_lowercase();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/api/providers.rs` around lines 785 - 829, The test handler normalizes
provider IDs (using provider_toml_key and model_matches_provider) but the
update/delete handlers still use raw provider strings, causing case-sensitivity
mismatches; update the /providers create/update/delete code to trim and
lowercase incoming provider IDs (same normalization logic used in the test flow)
before calling provider_toml_key, model_matches_provider, build_test_llm_config
or any save/remove operations so that all endpoints operate on the
normalized_provider value consistently.
interface/src/components/ModelSelect.tsx (1)

131-154: Fix provider-order fallback for unknown providers.

indexOf returns -1 (not null/undefined), so unknown providers currently sort to the top instead of the end.

♻️ Proposed fix
-  const sortedProviders = Object.keys(grouped).sort(
-    (a, b) =>
-      (providerOrder.indexOf(a) ?? 99) - (providerOrder.indexOf(b) ?? 99),
-  );
+  const providerRank = (provider: string) => {
+    const index = providerOrder.indexOf(provider);
+    return index === -1 ? Number.MAX_SAFE_INTEGER : index;
+  };
+  const sortedProviders = Object.keys(grouped).sort(
+    (a, b) => providerRank(a) - providerRank(b),
+  );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@interface/src/components/ModelSelect.tsx` around lines 131 - 154, The sort
fallback is wrong because providerOrder.indexOf(...) returns -1 for unknown
providers, so sortedProviders ends up placing unknown keys first; update the
comparator in the sortedProviders definition (which references providerOrder and
grouped) to treat indexOf(...) === -1 as a large rank (e.g., 99) — for both a
and b compute idxA = providerOrder.indexOf(a) === -1 ? 99 :
providerOrder.indexOf(a) and idxB similarly, then return idxA - idxB so unknown
providers sort to the end.
src/config.rs (1)

4495-4580: Provider menu/index mapping is still fragile

The array + manual match provider_idx approach is easy to desync (already happened once). A single metadata table (label, prompt, key, provider_id) would remove this class of bug.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/config.rs` around lines 4495 - 4580, The current two-part design
(providers array + match on provider_idx) is fragile; replace the providers
array and the match(provider_idx) with a single metadata table (e.g., a Vec or
slice of structs/tuples containing display_label, prompt_text, toml_key,
provider_id and any extra flags like supports_oauth) and then feed the
display_label list to Select to get provider_idx, and index into that same
metadata table to obtain prompt_text/toml_key/provider_id and oauth behavior
(instead of the match block). Update code paths that reference providers,
provider_idx, and the match (including the Anthropic OAuth branch) to use the
new metadata entries (e.g., lookup on metadata[provider_idx].supports_oauth /
.prompt_text / .toml_key / .provider_id).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/config.rs`:
- Around line 2901-2906: The kilo_key and opencode_go_key resolution currently
prefers the TOML value over environment variables; change the order so
environment vars take precedence: for kilo_key swap the checks to try
std::env::var("KILO_API_KEY").ok() first, then fall back to
toml.llm.kilo_key.as_deref().and_then(resolve_env_value), and do the analogous
change for opencode_go_key (use the corresponding OPENCODE_GO_KEY env var first,
then toml.llm.opencode_go_key with resolve_env_value). Ensure you only reorder
the existing calls (refer to kilo_key, opencode_go_key, resolve_env_value, and
toml.llm.*) so env > config precedence is enforced.

---

Outside diff comments:
In `@src/config.rs`:
- Around line 4791-4816: The EnvGuard's KEYS constant omits
"MINIMAX_CN_API_KEY", allowing host env values to leak into tests; add
"MINIMAX_CN_API_KEY" to the KEYS array defined in src/config.rs so EnvGuard will
strip that variable during test runs (update the const KEYS list where other API
key names like "MINIMAX_API_KEY" and "MOONSHOT_API_KEY" are declared).
- Around line 2399-2440: The needs_onboarding() logic omits checking for
MINIMAX_CN_API_KEY so if only that env var is present onboarding is still
required; update the has_legacy_keys and has_legacy_bootstrap_vars checks inside
needs_onboarding() to include std::env::var("MINIMAX_CN_API_KEY").is_ok() (add
the same check where legacy keys and bootstrap vars are enumerated) so the
function recognizes the MINIMAX_CN_API_KEY as a valid existing provider key
supported by load_from_env().

---

Nitpick comments:
In `@interface/src/components/ModelSelect.tsx`:
- Around line 131-154: The sort fallback is wrong because
providerOrder.indexOf(...) returns -1 for unknown providers, so sortedProviders
ends up placing unknown keys first; update the comparator in the sortedProviders
definition (which references providerOrder and grouped) to treat indexOf(...)
=== -1 as a large rank (e.g., 99) — for both a and b compute idxA =
providerOrder.indexOf(a) === -1 ? 99 : providerOrder.indexOf(a) and idxB
similarly, then return idxA - idxB so unknown providers sort to the end.

In `@src/api/providers.rs`:
- Around line 785-829: The test handler normalizes provider IDs (using
provider_toml_key and model_matches_provider) but the update/delete handlers
still use raw provider strings, causing case-sensitivity mismatches; update the
/providers create/update/delete code to trim and lowercase incoming provider IDs
(same normalization logic used in the test flow) before calling
provider_toml_key, model_matches_provider, build_test_llm_config or any
save/remove operations so that all endpoints operate on the normalized_provider
value consistently.

In `@src/config.rs`:
- Around line 4495-4580: The current two-part design (providers array + match on
provider_idx) is fragile; replace the providers array and the
match(provider_idx) with a single metadata table (e.g., a Vec or slice of
structs/tuples containing display_label, prompt_text, toml_key, provider_id and
any extra flags like supports_oauth) and then feed the display_label list to
Select to get provider_idx, and index into that same metadata table to obtain
prompt_text/toml_key/provider_id and oauth behavior (instead of the match
block). Update code paths that reference providers, provider_idx, and the match
(including the Anthropic OAuth branch) to use the new metadata entries (e.g.,
lookup on metadata[provider_idx].supports_oauth / .prompt_text / .toml_key /
.provider_id).

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b9bda6d and d802d64.

📒 Files selected for processing (14)
  • README.md
  • docs/content/docs/(configuration)/config.mdx
  • docs/content/docs/(deployment)/roadmap.mdx
  • docs/content/docs/(getting-started)/quickstart.mdx
  • interface/src/api/client.ts
  • interface/src/components/ModelSelect.tsx
  • interface/src/lib/providerIcons.tsx
  • interface/src/routes/Settings.tsx
  • src/api/models.rs
  • src/api/providers.rs
  • src/config.rs
  • src/llm/model.rs
  • src/llm/providers.rs
  • src/llm/routing.rs
🚧 Files skipped from review as they are similar to previous changes (6)
  • docs/content/docs/(getting-started)/quickstart.mdx
  • docs/content/docs/(configuration)/config.mdx
  • interface/src/routes/Settings.tsx
  • src/llm/routing.rs
  • interface/src/lib/providerIcons.tsx
  • README.md

Comment thread src/config.rs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
src/config.rs (1)

494-514: Reuse add_shorthand_provider for OpenCode providers to remove duplicate registration logic.

opencode-zen and opencode-go are still manually registered in both load_from_env and from_toml, even though the helper exists. Centralizing these two blocks reduces drift risk.

♻️ Suggested consolidation
-        if let Some(opencode_zen_key) = llm.opencode_zen_key.clone() {
-            llm.providers
-                .entry("opencode-zen".to_string())
-                .or_insert_with(|| ProviderConfig {
-                    api_type: ApiType::OpenAiCompletions,
-                    base_url: OPENCODE_ZEN_PROVIDER_BASE_URL.to_string(),
-                    api_key: opencode_zen_key,
-                    name: None,
-                    use_bearer_auth: false,
-                });
-        }
+        add_shorthand_provider(
+            &mut llm.providers,
+            "opencode-zen",
+            llm.opencode_zen_key.clone(),
+            ApiType::OpenAiCompletions,
+            OPENCODE_ZEN_PROVIDER_BASE_URL,
+            None,
+            false,
+        );

-        if let Some(opencode_go_key) = llm.opencode_go_key.clone() {
-            llm.providers
-                .entry("opencode-go".to_string())
-                .or_insert_with(|| ProviderConfig {
-                    api_type: ApiType::OpenAiCompletions,
-                    base_url: OPENCODE_GO_PROVIDER_BASE_URL.to_string(),
-                    api_key: opencode_go_key,
-                    name: None,
-                    use_bearer_auth: false,
-                });
-        }
+        add_shorthand_provider(
+            &mut llm.providers,
+            "opencode-go",
+            llm.opencode_go_key.clone(),
+            ApiType::OpenAiCompletions,
+            OPENCODE_GO_PROVIDER_BASE_URL,
+            None,
+            false,
+        );

Also applies to: 2571-2593, 3103-3125

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/config.rs` around lines 494 - 514, The code currently duplicates
registration of the "opencode-zen" and "opencode-go" providers in both
load_from_env and from_toml; replace those manual insertions by calling the
existing helper add_shorthand_provider with the same arguments (provider_id,
key, api_type, base_url, name, use_bearer_auth) so both code paths use a single
centralized registration path; locate the manual insertions for "opencode-zen"
and "opencode-go" in the blocks around load_from_env and from_toml and swap them
for calls to add_shorthand_provider(providers, "opencode-zen", ... ) and
add_shorthand_provider(providers, "opencode-go", ... ) to remove duplication and
prevent drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/api/providers.rs`:
- Around line 787-789: The code validates and later uses request.model without
trimming, causing whitespace to break provider model lookup and connectivity
tests; create a trimmed version (e.g., let normalized_model =
request.model.trim().to_string()) and replace uses of request.model in the
validation block and in the model construction/test invocation with
normalized_model (similar to how normalized_provider is used), ensuring all
comparisons and the ProviderModel::new/ProviderModelTest invocation use the
trimmed value.

In `@src/llm/model.rs`:
- Around line 750-754: In call_openai_compatible, remove the
ApiType::KiloGateway arm from the endpoint_path match so the match only includes
providers that actually invoke call_openai_compatible (e.g., Gemini and OpenAI
variants); KiloGateway is handled separately by
call_openai_compatible_with_optional_auth with a hardcoded endpoint, so delete
the KiloGateway branch from the match expression that computes endpoint_path to
eliminate the dead code.

---

Nitpick comments:
In `@src/config.rs`:
- Around line 494-514: The code currently duplicates registration of the
"opencode-zen" and "opencode-go" providers in both load_from_env and from_toml;
replace those manual insertions by calling the existing helper
add_shorthand_provider with the same arguments (provider_id, key, api_type,
base_url, name, use_bearer_auth) so both code paths use a single centralized
registration path; locate the manual insertions for "opencode-zen" and
"opencode-go" in the blocks around load_from_env and from_toml and swap them for
calls to add_shorthand_provider(providers, "opencode-zen", ... ) and
add_shorthand_provider(providers, "opencode-go", ... ) to remove duplication and
prevent drift.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d802d64 and bcb6daa.

📒 Files selected for processing (14)
  • README.md
  • docs/content/docs/(configuration)/config.mdx
  • docs/content/docs/(deployment)/roadmap.mdx
  • docs/content/docs/(getting-started)/quickstart.mdx
  • interface/src/api/client.ts
  • interface/src/components/ModelSelect.tsx
  • interface/src/lib/providerIcons.tsx
  • interface/src/routes/Settings.tsx
  • src/api/models.rs
  • src/api/providers.rs
  • src/config.rs
  • src/llm/model.rs
  • src/llm/providers.rs
  • src/llm/routing.rs
🚧 Files skipped from review as they are similar to previous changes (6)
  • README.md
  • interface/src/routes/Settings.tsx
  • interface/src/api/client.ts
  • src/llm/providers.rs
  • docs/content/docs/(configuration)/config.mdx
  • docs/content/docs/(getting-started)/quickstart.mdx

Comment thread src/api/providers.rs
Comment thread src/llm/model.rs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/api/providers.rs (1)

712-764: ⚠️ Potential issue | 🟡 Minor

Trim API keys before persisting provider config.

Line 721 validates a trimmed key, but Line 763 writes request.api_key untrimmed. Whitespace-only edges can pass validation and still break auth at runtime.

🔧 Proposed fix
 pub(super) async fn update_provider(
     State(state): State<Arc<ApiState>>,
     Json(request): Json<ProviderUpdateRequest>,
 ) -> Result<Json<ProviderUpdateResponse>, StatusCode> {
     let normalized_provider = request.provider.trim().to_lowercase();
     let normalized_model = request.model.trim();
+    let normalized_api_key = request.api_key.trim();
     let Some(key_name) = provider_toml_key(&normalized_provider) else {
         return Ok(Json(ProviderUpdateResponse {
             success: false,
             message: format!("Unknown provider: {}", request.provider),
         }));
     };

-    if request.api_key.trim().is_empty() {
+    if normalized_api_key.is_empty() {
         return Ok(Json(ProviderUpdateResponse {
             success: false,
             message: "API key cannot be empty".into(),
         }));
     }
@@
-    doc["llm"][key_name] = toml_edit::value(request.api_key);
+    doc["llm"][key_name] = toml_edit::value(normalized_api_key);
     apply_model_routing(&mut doc, normalized_model);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/api/providers.rs` around lines 712 - 764, The API key is validated with
trimming but the untrimmed request.api_key is stored; change the code that
writes to the TOML (the assignment to doc["llm"][key_name]) to persist a trimmed
key (e.g., call .trim() and use that trimmed value or create a trimmed variable
like trimmed_api_key) so whitespace around keys is removed before saving; locate
the assignment to doc["llm"][key_name] in the same function that uses
provider_toml_key, normalized_provider/normalized_model, and apply_model_routing
and replace the stored value with the trimmed API key.
🧹 Nitpick comments (1)
src/config.rs (1)

2519-2677: Remove duplicate provider-registration blocks in load_from_env.

Lines 2519-2601 and Lines 2615-2677 both register overlapping providers (openrouter, kilo, zhipu, zai-coding-plan, opencode-*, minimax*). The second pass is effectively a no-op because of or_insert_with, but it adds drift risk.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/config.rs` around lines 2519 - 2677, The file has duplicated
provider-registration logic inside load_from_env: remove the redundant second
block so each provider is registered only once; specifically delete the repeated
entries that re-add "openrouter", "kilo", "zhipu", "zai-coding-plan",
"opencode-zen", "opencode-go", "minimax", "minimax-cn", and "openai" which
duplicate earlier calls to add_shorthand_provider and
llm.providers.entry(...).or_insert_with(...) (i.e., keep a single consistent
registration flow using add_shorthand_provider and ProviderConfig/or_insert_with
calls and ensure no duplicate provider keys remain).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/api/providers.rs`:
- Around line 712-764: The API key is validated with trimming but the untrimmed
request.api_key is stored; change the code that writes to the TOML (the
assignment to doc["llm"][key_name]) to persist a trimmed key (e.g., call .trim()
and use that trimmed value or create a trimmed variable like trimmed_api_key) so
whitespace around keys is removed before saving; locate the assignment to
doc["llm"][key_name] in the same function that uses provider_toml_key,
normalized_provider/normalized_model, and apply_model_routing and replace the
stored value with the trimmed API key.

---

Nitpick comments:
In `@src/config.rs`:
- Around line 2519-2677: The file has duplicated provider-registration logic
inside load_from_env: remove the redundant second block so each provider is
registered only once; specifically delete the repeated entries that re-add
"openrouter", "kilo", "zhipu", "zai-coding-plan", "opencode-zen", "opencode-go",
"minimax", "minimax-cn", and "openai" which duplicate earlier calls to
add_shorthand_provider and llm.providers.entry(...).or_insert_with(...) (i.e.,
keep a single consistent registration flow using add_shorthand_provider and
ProviderConfig/or_insert_with calls and ensure no duplicate provider keys
remain).

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bcb6daa and 850b43f.

📒 Files selected for processing (3)
  • src/api/providers.rs
  • src/config.rs
  • src/llm/model.rs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/llm/model.rs`:
- Around line 750-753: The match arm for provider_config.api_type is incorrectly
treating ApiType::OpenAiResponses as a chat-completions endpoint; update the
match in the chat-completions helper inside model.rs so ApiType::OpenAiResponses
maps to the Responses endpoint (e.g., "/v1/responses") rather than
"/v1/chat/completions", and ensure the other arm(s) still map
ApiType::OpenAiChatCompletions (and ApiType::Gemini) to "/chat/completions";
adjust the same logic for the corresponding block at lines ~758-763 so
OpenAiResponses is always guarded and routed to the correct responses endpoint.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 850b43f and e620cb5.

📒 Files selected for processing (4)
  • docs/content/docs/(configuration)/config.mdx
  • interface/src/api/client.ts
  • interface/src/routes/Settings.tsx
  • src/llm/model.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • interface/src/api/client.ts

Comment thread src/llm/model.rs
Comment on lines 750 to 753
let endpoint_path = match provider_config.api_type {
ApiType::OpenAiCompletions | ApiType::OpenAiResponses => "/v1/chat/completions",
ApiType::Gemini => "/chat/completions",
ApiType::OpenAiChatCompletions | ApiType::Gemini => "/chat/completions",
ApiType::Anthropic => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Guard OpenAiResponses in chat-completions helper to prevent wrong endpoint use.

Line 751 currently maps ApiType::OpenAiResponses to /v1/chat/completions, which conflicts with the Responses API contract (/v1/responses). Even if currently not hit, this is a fragile footgun for future callers.

🛠️ Suggested fix
-        let endpoint_path = match provider_config.api_type {
-            ApiType::OpenAiCompletions | ApiType::OpenAiResponses => "/v1/chat/completions",
+        let endpoint_path = match provider_config.api_type {
+            ApiType::OpenAiCompletions => "/v1/chat/completions",
             ApiType::OpenAiChatCompletions | ApiType::Gemini => "/chat/completions",
+            ApiType::OpenAiResponses => {
+                return Err(CompletionError::ProviderError(format!(
+                    "{provider_display_name} uses openai_responses; call_openai_responses must be used instead"
+                )));
+            }
             ApiType::Anthropic => {
                 return Err(CompletionError::ProviderError(format!(
                     "{provider_display_name} is configured with anthropic API type, but this call expects an OpenAI-compatible API"
                 )));
             }

Also applies to: 758-763

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/llm/model.rs` around lines 750 - 753, The match arm for
provider_config.api_type is incorrectly treating ApiType::OpenAiResponses as a
chat-completions endpoint; update the match in the chat-completions helper
inside model.rs so ApiType::OpenAiResponses maps to the Responses endpoint
(e.g., "/v1/responses") rather than "/v1/chat/completions", and ensure the other
arm(s) still map ApiType::OpenAiChatCompletions (and ApiType::Gemini) to
"/chat/completions"; adjust the same logic for the corresponding block at lines
~758-763 so OpenAiResponses is always guarded and routed to the correct
responses endpoint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants