Repository navigation
feat: dark/light theme toggle and live provider model list - #682
Merged
Merged
Conversation
Server: new GET /providers/{provider}/models endpoint that lists models
through the provider's OpenAI-compatible client (10s timeout, fresh
short-lived plugin instance per request, closes its client). Failures
(no key, network, non-OpenAI provider) return source=unavailable with an
empty list — never 500. Contract doc updated.
Webui: light theme overrides for the CSS custom properties with a
localStorage-persisted toggle in the header (dark stays default,
pre-render init avoids flash). The model input's datalist now fetches
live model ids for the selected provider (debounced, stale requests
aborted), falling back to the built-in presets when unavailable; input
stays free-text.
Tests: 8 endpoint tests + 33 vitest (6 new); ruff clean.
Contributor
📝 WalkthroughWalkthroughThe API now lists provider models with timeout and fallback behavior, and the WebUI displays live model options while retaining presets. The WebUI also gains persistent dark/light theme selection with corresponding styling and tests. ChangesProvider model listing
Persistent WebUI themes
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant WebUI as AgentClient
participant API as Provider models endpoint
participant Helper as list_provider_models
participant Provider as OpenAICompatProvider
participant Remote as Provider /models API
WebUI->>API: GET /providers/{provider}/models
API->>Helper: list_provider_models(provider)
Helper->>Provider: provide_llm(provider)
Helper->>Provider: list_models()
Provider->>Remote: GET /models
Remote-->>Provider: model records
Provider-->>Helper: model IDs
Helper-->>API: model IDs or timeout/error
API-->>WebUI: live or unavailable response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
coding-agent/webui/app/src/App.tsx (1)
31-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse one shared theme-storage key.
The same string is maintained in two files; drift could make the pre-rendered theme and React persistence use different keys.
coding-agent/webui/app/src/App.tsx#L31-L32: export the key from a shared module.coding-agent/webui/app/src/main.tsx#L8-L9: import and use that shared key instead of the literal.🤖 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 `@coding-agent/webui/app/src/App.tsx` around lines 31 - 32, The theme-storage key is duplicated between App.tsx and main.tsx; export THEME_LS_KEY from the shared module in coding-agent/webui/app/src/App.tsx (lines 31-32), then import and use that exported key in coding-agent/webui/app/src/main.tsx (lines 8-9) instead of the literal.
🤖 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 `@coding-agent/src/coding_agent/server/provider_models.py`:
- Around line 35-42: Ensure provider instances created by provide_llm() are
closed even when they are not OpenAICompatProvider: restructure the flow around
the isinstance check so cleanup runs before returning an empty list, while
preserving the existing compatible-provider timeout and close behavior. Update
coding-agent/src/coding_agent/server/provider_models.py lines 35-42 accordingly;
update coding-agent/tests/ui/test_provider_models.py lines 144-151 to use a
non-compatible fake provider and assert that it is closed.
In `@coding-agent/webui/app/src/components/Header.tsx`:
- Around line 49-79: Reset liveModels to null immediately whenever
config.provider changes, before scheduling the debounced listProviderModels
request. Update the useEffect handling provider changes so switching directly
between non-empty providers falls back to MODEL_PRESETS during the debounce and
fetch period, while preserving the existing abort and response handling.
---
Nitpick comments:
In `@coding-agent/webui/app/src/App.tsx`:
- Around line 31-32: The theme-storage key is duplicated between App.tsx and
main.tsx; export THEME_LS_KEY from the shared module in
coding-agent/webui/app/src/App.tsx (lines 31-32), then import and use that
exported key in coding-agent/webui/app/src/main.tsx (lines 8-9) instead of the
literal.
🪄 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: 8902b8cd-90ae-4c65-a3e8-55c9357ae0be
📒 Files selected for processing (13)
coding-agent/docs/web-client-api-contract.mdcoding-agent/src/coding_agent/providers/openai_compat.pycoding-agent/src/coding_agent/server/http_server.pycoding-agent/src/coding_agent/server/provider_models.pycoding-agent/src/coding_agent/server/schemas.pycoding-agent/tests/ui/test_provider_models.pycoding-agent/webui/app/src/App.test.tsxcoding-agent/webui/app/src/App.tsxcoding-agent/webui/app/src/components/Header.tsxcoding-agent/webui/app/src/index.csscoding-agent/webui/app/src/lib/api.tscoding-agent/webui/app/src/lib/types.tscoding-agent/webui/app/src/main.tsx
Comment on lines
+35
to
+42
| plugin = LLMProviderPlugin(provider=provider, model="", api_key="") | ||
| instance = plugin.provide_llm() | ||
| if not isinstance(instance, OpenAICompatProvider): | ||
| return [] | ||
| try: | ||
| return await asyncio.wait_for(instance.list_models(), timeout=timeout) | ||
| finally: | ||
| await instance.close() |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg 'coding-agent/(src|tests)/.*provider_models' || true
echo "== provider_models.py outline/section =="
if [ -f coding-agent/src/coding_agent/server/provider_models.py ]; then
wc -l coding-agent/src/coding_agent/server/provider_models.py
cat -n coding-agent/src/coding_agent/server/provider_models.py
fi
echo "== test section =="
if [ -f coding-agent/tests/ui/test_provider_models.py ]; then
wc -l coding-agent/tests/ui/test_provider_models.py
sed -n '120,175p' coding-agent/tests/ui/test_provider_models.py | cat -n
fi
echo "== related provider plugin/class definitions =="
rg -n "class LLMProviderPlugin|provide_llm|class OpenAICompatProvider|close\\(" coding-agent -S || trueRepository: delete-cloud/Coding-Agent-Loop
Length of output: 36514
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate llm provider and compatibility base/provider implementations =="
git ls-files 'coding-agent/src/coding_agent/plugins/llm_provider.py' 'coding-agent/src/coding_agent/providers/*.py' | xargs -r rg -n 'class .*Provider|class .*Compat|def close|provide_llm|plugin_manager'
echo "== llm_provider implementation =="
cat -n coding-agent/src/coding_agent/plugins/llm_provider.py
echo "== provider implementations with close =="
for f in coding-agent/src/coding_agent/providers/*.py; do
if rg -q 'class .*Provider|async def close|def close' "$f"; then
echo "--- $f ---"
wc -l "$f"
rg -n -C 4 'class .*Provider|async def close|def close|def __init__|http_client' "$f" | sed -n '1,220p' || true
fi
doneRepository: delete-cloud/Coding-Agent-Loop
Length of output: 15831
Close non-compatible provider instances before returning an empty list. provide_llm() constructs instances before isinstance(instance, OpenAICompatProvider) checks them, so non-compatible providers bypass finally and keep their short-lived cleanup resources alive.
coding-agent/src/coding_agent/server/provider_models.py#L35-L42: return the non-compatible provider to cleanup instead of returning early, or skip construction via an explicit OpenAI-compatible provider-only path.coding-agent/tests/ui/test_provider_models.py#L144-L151: replace the sentinel with a non-compatible provider/fake and assert it is closed.
📍 Affects 2 files
coding-agent/src/coding_agent/server/provider_models.py#L35-L42(this comment)coding-agent/tests/ui/test_provider_models.py#L144-L151
🤖 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 `@coding-agent/src/coding_agent/server/provider_models.py` around lines 35 -
42, Ensure provider instances created by provide_llm() are closed even when they
are not OpenAICompatProvider: restructure the flow around the isinstance check
so cleanup runs before returning an empty list, while preserving the existing
compatible-provider timeout and close behavior. Update
coding-agent/src/coding_agent/server/provider_models.py lines 35-42 accordingly;
update coding-agent/tests/ui/test_provider_models.py lines 144-151 to use a
non-compatible fake provider and assert that it is closed.
Comment on lines
+49
to
+79
| client, | ||
| theme, | ||
| onToggleTheme, | ||
| }: Props) { | ||
| // Live model ids for the selected provider; null = use MODEL_PRESETS. | ||
| const [liveModels, setLiveModels] = useState<string[] | null>(null); | ||
|
|
||
| useEffect(() => { | ||
| const provider = config.provider.trim(); | ||
| if (!provider) { | ||
| setLiveModels(null); | ||
| return; | ||
| } | ||
| const ctrl = new AbortController(); | ||
| // Debounce rapid provider switches; the abort drops superseded requests. | ||
| const timer = setTimeout(() => { | ||
| client | ||
| .listProviderModels(provider, ctrl.signal) | ||
| .then((res) => { | ||
| setLiveModels(res.source === "live" && res.models.length > 0 ? res.models : null); | ||
| }) | ||
| .catch(() => setLiveModels(null)); | ||
| }, 250); | ||
| return () => { | ||
| clearTimeout(timer); | ||
| ctrl.abort(); | ||
| }; | ||
| }, [client, config.provider]); | ||
|
|
||
| const modelOptions = liveModels ?? MODEL_PRESETS; | ||
|
|
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Stale model list shown briefly when switching providers.
liveModels is only reset to null when the provider is cleared (Line 59) or after the new fetch resolves/rejects (Lines 68-70). When switching directly from one non-empty provider to another, the previous provider's live model ids remain in modelOptions for the debounce delay + fetch duration, so the datalist can momentarily offer model ids that don't belong to the newly selected provider.
🐛 Proposed fix
useEffect(() => {
const provider = config.provider.trim();
if (!provider) {
setLiveModels(null);
return;
}
+ // Avoid showing the previous provider's (now mismatched) model list
+ // while the new provider's fetch is in flight.
+ setLiveModels(null);
const ctrl = new AbortController();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| client, | |
| theme, | |
| onToggleTheme, | |
| }: Props) { | |
| // Live model ids for the selected provider; null = use MODEL_PRESETS. | |
| const [liveModels, setLiveModels] = useState<string[] | null>(null); | |
| useEffect(() => { | |
| const provider = config.provider.trim(); | |
| if (!provider) { | |
| setLiveModels(null); | |
| return; | |
| } | |
| const ctrl = new AbortController(); | |
| // Debounce rapid provider switches; the abort drops superseded requests. | |
| const timer = setTimeout(() => { | |
| client | |
| .listProviderModels(provider, ctrl.signal) | |
| .then((res) => { | |
| setLiveModels(res.source === "live" && res.models.length > 0 ? res.models : null); | |
| }) | |
| .catch(() => setLiveModels(null)); | |
| }, 250); | |
| return () => { | |
| clearTimeout(timer); | |
| ctrl.abort(); | |
| }; | |
| }, [client, config.provider]); | |
| const modelOptions = liveModels ?? MODEL_PRESETS; | |
| client, | |
| theme, | |
| onToggleTheme, | |
| }: Props) { | |
| // Live model ids for the selected provider; null = use MODEL_PRESETS. | |
| const [liveModels, setLiveModels] = useState<string[] | null>(null); | |
| useEffect(() => { | |
| const provider = config.provider.trim(); | |
| if (!provider) { | |
| setLiveModels(null); | |
| return; | |
| } | |
| // Avoid showing the previous provider's (now mismatched) model list | |
| // while the new provider's fetch is in flight. | |
| setLiveModels(null); | |
| const ctrl = new AbortController(); | |
| // Debounce rapid provider switches; the abort drops superseded requests. | |
| const timer = setTimeout(() => { | |
| client | |
| .listProviderModels(provider, ctrl.signal) | |
| .then((res) => { | |
| setLiveModels(res.source === "live" && res.models.length > 0 ? res.models : null); | |
| }) | |
| .catch(() => setLiveModels(null)); | |
| }, 250); | |
| return () => { | |
| clearTimeout(timer); | |
| ctrl.abort(); | |
| }; | |
| }, [client, config.provider]); | |
| const modelOptions = liveModels ?? MODEL_PRESETS; |
🤖 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 `@coding-agent/webui/app/src/components/Header.tsx` around lines 49 - 79, Reset
liveModels to null immediately whenever config.provider changes, before
scheduling the debounced listProviderModels request. Update the useEffect
handling provider changes so switching directly between non-empty providers
falls back to MODEL_PRESETS during the debounce and fetch period, while
preserving the existing abort and response handling.
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
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.
改动
主题切换:light 主题复用同一套 CSS 自定义属性做覆盖,header 加切换按钮,localStorage 持久化,渲染前初始化避免闪烁,默认仍为暗色。
模型列表:新增
GET /providers/{provider}/models(已实测 kimi-code 返回 kimi-for-coding / kimi-for-coding-highspeed / k3 / k3-256k)。前端 model 输入的 datalist 改为拉取实时列表,失败回退内置预设,保留自由输入。测试
Summary by CodeRabbit
New Features
Documentation
Tests