[lejyfps]

Fixes #1194.

Adds API Route as an explicitly selectable provider in Settings > AI. Users can save a dedicated key, refresh their account's model catalog and select a model for SQL generation, explanation and the existing AI naming features. Existing provider choices and keys are preserved.

The backend reuses the existing OpenAI-compatible Chat Completions transport at https://global.api-route.com/v1. Model discovery calls the authenticated /models endpoint in both fresh and cached catalog paths, using API Route's own proxy configuration. No fixed model list, new inference SDK or automatic provider selection is introduced. Credentials use the existing secret store, with API_ROUTE_API_KEY as an environment fallback on desktop and web.

Failed API Route model refreshes (network, HTTP or response errors) retain the last successful cached catalog, including forced refreshes. Successful empty responses and missing keys still clear the catalog.

Validation:

• 33 focused frontend tests passed, including explicit provider selection, proxy settings and model refresh.
• Frontend typecheck, ESLint for changed source files, Prettier, rustfmt and git diff --check passed.
• Added Rust wire tests for authenticated model discovery, missing keys, authentication errors, malformed catalog responses, exact model IDs, Chat Completions and upstream errors. Fixtures terminate traffic locally.
• Attempted cargo test --no-default-features --lib ai::provider_tests --offline and cargo check --no-default-features --offline. Both stop in the OpenSSL dependency build because the local Perl installation lacks Locale::Maketext::Simple. Rust tests have not run locally. The follow-up regression run and no-default-features check also waited on a build-directory lock held by another Cargo build and were stopped without touching that build.
• GitNexus impact/detect_changes were attempted but the native LadybugDB module fails to load. Callers and the complete diff were inspected manually.
• No live authenticated API Route request was made: no test account key was available. Endpoint and Bearer authentication were checked against the provider's published documentation at https://www.api-route.com/.

Manual verification: in Settings > AI select API Route, save a test key (or set API_ROUTE_API_KEY), refresh models and select a chat model. Generate and explain a SQL query. Verify failed refreshes preserve a previously cached model list and an invalid key still prevents generation, API Route proxy overrides apply, and switching back preserves the other provider's configuration. API calls may incur account charges.

---

^{Open in [code]smith · Need help on this PR? Tag @codesmith-bot with what you need.}

[kilo-code-bot]

WARNING: Transient failures blank the API Route model list

api_route::models swallows every error (network failure, 401, 5xx, malformed body) and returns an empty Vec. Here that result is inserted unconditionally, so a momentary network blip or rate limit replaces the cached list with [], and the dropdown in Settings > AI empties out. The sibling Anthropic and MiniMax refreshes right above only overwrite the cache entry when the fetch returns a non-empty list. The same unconditional insert happens on the fresh path at line 572, and save_cache then persists the empty list. Consider keeping the previous cached entry when the fetch yields no models, and only inserting when it is non-empty.

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Incremental review of 8178c410..69be6c93. The new commits (46110c1b, 69be6c93) address the earlier reviewer's P2 finding: api_route::models_with_cache now persists a successful warm-path refresh into the on-disk cache entry. It does so under MODEL_CACHE_LOCK, keeps the shared last_updated timestamp, and falls back to the latest cached list on transient failure. The lock is only held in the synchronous section after the network await, so it is never held across an await point. A forced refresh that completes while a warm refresh is in flight is preserved, and a successful empty list still yields an empty list. The new tests cover the overlapping-refresh case and the success-then-failure sequence (200, 503, empty, 503). The remaining files in the incremental range are merged content from main.

Files Reviewed (4 files in the PR-owned increment)

• src-tauri/src/ai/api_route.rs
• src-tauri/src/ai.rs
• src-tauri/src/ai_provider_tests.rs
• src-tauri/src/application/ai.rs

Previous Review Summaries (2 snapshots, latest commit 8178c41)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 8178c41)

Status: No Issues Found | Recommendation: Merge

Incremental review of ebc21c37..8178c410: the previously reported WARNING (transient failures blanking the API Route model list) is resolved. api_route::models now loads the cached api-route entry and models_with_client returns it on network errors, non-success HTTP statuses, and malformed bodies. A successful empty data list still yields an empty list, which is the intended behavior. The new tests cover 401, 503, malformed-body, empty-list, and network-timeout cases.

Files Reviewed (2 files in this increment)

• src-tauri/src/ai/api_route.rs
• src-tauri/src/ai_provider_tests.rs

Previous review (commit ebc21c3)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	0

Issue Details (click to expand)

WARNING

File	Line	Issue
src-tauri/src/ai.rs	465	api_route::models swallows all errors and returns an empty list, which is inserted unconditionally in both the cached path (line 465) and the fresh path (line 572, then persisted by save_cache). A transient network or auth failure blanks the API Route model dropdown, unlike the Anthropic and MiniMax refreshes, which only overwrite on a non-empty result.

Files Reviewed (11 files)

• README.md
• packages/web-ui/src/components/settings/AiTab.tsx
• packages/web-ui/src/contexts/SettingsContext.ts
• packages/web-ui/src/utils/settingsUI.ts
• packages/web-ui/tests/components/settings/AiTab.test.tsx
• packages/web-ui/tests/utils/settingsUI.test.ts
• src-tauri/src/ai.rs - 1 issue
• src-tauri/src/ai/api_route.rs
• src-tauri/src/ai_provider_tests.rs
• src-tauri/src/application/ai.rs
• src-tauri/src/transport/desktop/config.rs

Fix these issues in Kilo Cloud

---

_{Reviewed by claude-sonnet-5.5 · Input: 248 · Output: 2.1K · Cached: 273.8K}

_{Review guidance: REVIEW.md from base branch main}

[1-DARK]

Thanks for the PR and for the detailed validation notes.

I reviewed the changes and verified them locally:

• The Rust tests pass: cargo test --no-default-features --lib ai:: (17/17).
• The desktop build compiles successfully with cargo check --lib.
• The affected frontend test suites pass (99/99).
• Typecheck, ESLint, Prettier, and rustfmt are clean on the changed files.
• No Clippy warnings were found in the changed files.
• The branch merges cleanly onto the current main.

The implementation follows the existing provider patterns, including shared key storage with an environment variable fallback, per-provider proxy overrides, OpenAI-compatible transport, and no automatic provider selection.

LGTM - no blocking issues found.

[NewtTheWolf]

Thanks for reusing the shared OpenAI-compatible transport and keeping API Route an explicit provider choice. I reviewed 8178c410297dd06c71171b10ecc6af58c0946745: 33 targeted frontend tests and all ten Rust provider-wire tests passed, including model-cache fallback and exact account model IDs. Typecheck, targeted source ESLint and the production frontend build passed. The endpoint and Bearer scheme agree with the provider's published setup.

I found no evident issue in this pass. I will check the real-app selection, model refresh, generation/error paths and provider switching, then run the full required checks before merging. No live paid request has been made; any local fixture coverage will be called out separately.

[NewtTheWolf]

The real-app verification of 8178c410297dd06c71171b10ecc6af58c0946745 found a cache regression, so I am holding this open. pnpm lint && pnpm typecheck && pnpm test:all passed, including 8,657 frontend tests. Selection, Bearer authentication, the provider proxy, SQL generation/execution and explanation worked through the actual app/Rust transport against a local TLS/CONNECT fixture.

Reproduction:

1. Explicitly refresh API Route models successfully to cache catalog A (account-chat-model).
2. Let the provider return catalog B (updated-account-model), then reopen AI settings while the global cache is fresh. The non-forced refresh offers B; select that model.
3. Make the provider return HTTP 503 and click Refresh Models. The dropdown reverts to A, and the selected B model is marked not found.

Expected: retain B, the last successful catalog. Actual: ai_models_cache.json still contains A after step 2. The new warm-cache branch inserts the successful API Route result into a local map and returns without persisting it; the next api_route::models call reloads the older disk entry for its fallback.

Please persist successful API Route catalog updates on both paths, preserving unrelated providers' cache timing, and add a warm-success-to-failed-forced-refresh regression. The current helper tests inject the fallback list directly and do not cover this lifecycle. GitHub also now reports conflicts with main, so this needs a rebase before the next verification.

All provider traffic in this reproduction terminated locally, using a process-local test CA and isolated secret store. No live paid API request was made.

[NewtTheWolf]

[P2] Persist successful API Route catalog updates from the warm-cache path. This inserts the refreshed list into cached_models and immediately returns it without updating disk. I reproduced catalog A cached by a forced refresh, catalog B successfully loaded and selected through a later non-forced refresh, then an HTTP 503 causing the next forced refresh to restore A and mark B not found. api_route::models reads A from disk as its fallback. Retain the last successful B catalog on both paths and cover this success-then-failure sequence.

[1-DARK]

Thanks for the update and for merging in #1457. The branch still merges cleanly with current main. cargo test --no-default-features --lib ai:: passes 25/25 (21 on main), and the frontend suites, typecheck, rustfmt, and Clippy are clean for the changed files. Keeping the API Route key-required after #1457 is correct.

One change needed before merging, in the new models_with_cache (src-tauri/src/ai/api_route.rs:24-38). The cache file is read before the /models request, and the whole snapshot is written back after it. If a forced refresh finishes in between, its results for all providers and its timestamp get overwritten with the stale snapshot.

A test reproduces this: after a concurrent save_cache, the cache held openai=["fresh-model"] and the current timestamp; after the warm call returned, it was back to ["stale-model"] and last_updated=123.

In the app, I'd expect this when someone opens Settings > AI and clicks Refresh before API Route responds, though I haven't reproduced that path in the UI.

Suggested fix: Call load_cache again after the .await and update only the API Route entry before writing, plus add a regression test asserting that a concurrent save_cache survives.

Optional, non-blocking:

• detectAiDefaults auto-selects the first /models entry for API Route, which could be a non-chat model. You could leave API Route out of that auto-fill, since the backend already requires an explicit model.
• A brand icon instead of the generic Network icon.
• Next time, a separate commit instead of putting changes inside a merge commit would make review easier.

Thanks again for the careful work on this!

[NewtTheWolf]

Thanks for persisting successful warm-cache discoveries. The new sequential regression covers my previous catalog-B/failing-refresh case, and that correction addresses the lost API Route catalog I reported.

I reproduced the separate cache race reported by @1-DARK on 46110c1. models_with_cache reads the whole shared cache before awaiting the account's /models request, then writes that old snapshot back. In a local TLS fixture I paused the request after its first poll, wrote a concurrent refresh with openai: ["fresh-openai"] and timestamp 456, then completed API Route's response. The persisted cache contained the earlier openai: ["old-openai"] and timestamp 123 instead. A provider-specific refresh must preserve that newer unrelated catalog and timestamp.

Please re-read and merge into the latest shared cache after the network request, with synchronization covering the read/merge/write operation. Add an overlapping-refresh regression, including a failed API Route request, so neither path can restore a stale whole-cache snapshot. The current sequential test alone does not cover this interleaving.

Typecheck and the AI frontend tests pass. I am keeping this open for the reproduced cache overwrite.

[NewtTheWolf]

Re-reviewed 69be6c936a8b014ddf28c40c8158464144d98eb3. The reproduced cache overwrite is fixed: model discovery finishes before taking the shared cache lock, then reads and updates the latest cache while preserving unrelated catalogs and its timestamp. Failed discovery returns the latest cached API Route list without writing an old snapshot.

The original independent TLS race probe that failed on 46110c1b now passes unchanged: a concurrent OpenAI refresh and timestamp 456 survive the delayed API Route response. The 14 provider tests (cargo test --lib ai::provider_tests), including overlapping success, HTTP failure, malformed response and successful empty-list cases, pass, as do all 33 focused frontend tests, typecheck and lint. An earlier filter selected zero provider tests; that empty run is not counted as verification. No live API Route account was used.

@1-DARK, please re-review this commit against your outstanding cache-race review. My corresponding objection is resolved; I am leaving your review in place for your confirmation rather than dismissing it.