Skip to content

Fix UI improvements and AI configuration error handling#114

Merged
AmintaCCCP merged 13 commits intoAmintaCCCP:mainfrom
SummerRay160:Dev
Apr 27, 2026
Merged

Fix UI improvements and AI configuration error handling#114
AmintaCCCP merged 13 commits intoAmintaCCCP:mainfrom
SummerRay160:Dev

Conversation

@SummerRay160
Copy link
Copy Markdown
Contributor

@SummerRay160 SummerRay160 commented Apr 26, 2026

Summary by CodeRabbit

  • New Features

    • AI connection test now returns detailed, user-facing results including status and guidance.
  • Bug Fixes

    • AI analysis now blocks when API keys are empty or undecryptable, avoiding failed runs.
    • Bulk AI config import skips entries without usable keys and logs skipped items with reasons.
  • Improvements

    • Unified, clearer error messages for AI config failures across the UI.
    • Tweaks to markdown image error/loading UI and GitHub link dark-mode styling.

HappySummer and others added 12 commits April 25, 2026 21:28
修复持久化状态中的主题设置问题,不再强制使用暗色主题
改进发现页面的滚动行为,添加侧栏固定功能
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
移除对 scrollContainerRef 的依赖,统一使用 window 的滚动位置 API
简化滚动位置保存和恢复逻辑,提高代码一致性
添加暗黑模式下的背景和边框颜色样式,提升夜间使用的视觉体验
调整多个组件中的颜色样式,将品牌色和状态色统一为中性灰调色板,提升视觉一致性。主要修改包括:
- 替换品牌色和状态色为中性灰
- 统一按钮、图标、卡片等元素的颜色样式
- 优化暗黑模式下的颜色对比度
- 更新类型定义,使用 SubscriptionRepo 和 SubscriptionChannel 替代原有类型
- 重构 assetFilters 的状态管理,改用 useAppStore.setState
- 添加 searchHistoryVersion 状态用于跟踪搜索历史变化
- 优化删除操作的确认提示文案和翻译
- 改进 UI 交互细节和样式
- 为 SliderInput 组件添加 showMarks 属性控制标记显示
- 更新 BulkActionToolbar 按钮颜色以符合新设计规范
- 调整 BackToTop 和 ScrollToBottom 组件的背景和边框样式
- 简化 SearchBar 的 applyFilters 函数参数
- 移除 DiscoveryView 中未使用的 currentCount 和 isSidebarFixed
- 改进 MarkdownRenderer 的代码块样式和终端模拟器外观
- 修复 MarkdownImage 组件中文本透明度问题
调整Markdown渲染器中代码块的背景色透明度表示方式
优化数据管理面板中搜索历史版本的状态管理,移除不必要的依赖
- 在多个组件中添加对空API密钥状态的检查
- 改进AI服务连接测试的错误处理和反馈
- 优化API密钥批量更新时的空值处理
- 调整图片加载失败时的UI样式
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 26, 2026

📝 Walkthrough

Walkthrough

Server bulk AI-config insertion now skips entries lacking usable encrypted API keys and logs skipped items. Frontend AI flows treat both decrypt_failed and empty API-key states as invalid. aiService.testConnection() returns a structured ConnectionTestResult. Minor UI styling tweaks to image rendering and a GitHub button.

Changes

Cohort / File(s) Summary
API Key Status Validation
src/components/DiscoveryView.tsx, src/components/RepositoryCard.tsx, src/components/RepositoryList.tsx, src/components/SubscriptionRepoCard.tsx
AI analysis handlers now block when API key status is decrypt_failed OR empty; alert text updated to indicate both conditions.
AI Config Testing & Monitoring
src/services/aiService.ts, src/components/settings/AIConfigPanel.tsx
testConnection() now returns ConnectionTestResult (structured object with success/message/statusCode/statusText/errorType); AIConfigPanel updated to handle/display the new result shape and refined success/failure messaging.
Server-side Config Bulk Operations
server/src/routes/configs.ts
Bulk insert loop skips configs without a usable encrypted key, records reasons (masked-without-existing vs empty) into skippedConfigs, and emits a warning log listing skipped entries.
UI Component Styling
src/components/MarkdownRenderer.tsx, src/components/ReadmeModal.tsx, src/components/RepositoryCard.tsx
MarkdownImage error/loading UI compacted and restyled (smaller icon, denser layout, revised text/colors); ReadmeModal GitHub button dark-mode text/hover colors adjusted; Repository card alert text aligned with API key status changes.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Frontend Component
    participant AISvc as aiService
    participant Server as Backend API
    participant Crypto as Crypto Module

    Client->>AISvc: testConnection()
    AISvc->>Server: HTTP request (validated protocol/URL)
    alt Timeout
        Server--xAISvc: (timeout)
        AISvc->>AISvc: classify as timeout, build message
    else Network Error
        Server--xAISvc: (network error)
        AISvc->>AISvc: classify as network error, build message
    else HTTP Error
        Server--xAISvc: HTTP error (e.g., 401/403)
        AISvc->>AISvc: extract statusCode, map statusText, build message
    else Success
        Server-->>AISvc: 200 OK (content)
        AISvc->>AISvc: success = true, extract message
    end
    AISvc-->>Client: ConnectionTestResult { success, message, statusCode?, statusText?, errorType? }
    Client->>Client: Branch on result.success (show message / mark success)
Loading
sequenceDiagram
    participant Client as Frontend Caller
    participant API as Backend /api/configs/ai/bulk
    participant Crypto as Crypto Service
    participant DB as Database

    Client->>API: PUT /api/configs/ai/bulk (configs payload)
    loop For each config
        API->>Crypto: encrypt provided unmasked key OR request existing encrypted key
        alt encryptedKey available
            Crypto-->>API: encryptedKey
            API->>DB: INSERT config row with encryptedKey
        else no usable encryptedKey
            API->>API: add entry to skippedConfigs with reason (masked-without-existing | empty)
        end
    end
    API->>API: log warning listing skippedConfigs
    API-->>Client: respond (including skipped entries info)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 I hop through configs, sniffing each key,
If it's gone or masked, I skip with glee.
Tests tell tales with codes and light,
UIs whisper warnings, buttons shine bright.
A tiny hop, a tidy change — huzzah for me! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title references both UI improvements and AI configuration error handling, which accurately reflect the multiple changes across UI components and AI service configuration logic in this changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

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 (1)
server/src/routes/configs.ts (1)

141-176: ⚠️ Potential issue | 🟠 Major

Return accurate sync results when rows are skipped

Line 175 always reports all configs as synced, but Lines 151-160 can skip inserts. This can hide partial sync and make clients assume backend state is complete after the Line 134 full replace.

Proposed fix
-router.put('/api/configs/ai/bulk', (req, res) => {
+router.put('/api/configs/ai/bulk', (req, res) => {
   try {
     const db = getDb();
+    const skippedConfigs: Array<{ id: string; name: string; reason: string }> = [];
+    let insertedCount = 0;
     const configs = req.body.configs as Array<{
@@
-      const skippedConfigs: Array<{ id: string; name: string; reason: string }> = [];
-
       for (const c of configs) {
@@
         stmt.run(
           c.id, c.name ?? '', c.apiType ?? 'openai', c.baseUrl ?? '',
           encryptedKey, c.model ?? '', c.isActive ? 1 : 0,
           c.customPrompt ?? null, c.useCustomPrompt ? 1 : 0, c.concurrency ?? 1, c.reasoningEffort ?? null
         );
+        insertedCount++;
       }
@@
     });
 
     bulkSync();
-    res.json({ synced: configs.length });
+    res.json({ synced: insertedCount, skipped: skippedConfigs });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/src/routes/configs.ts` around lines 141 - 176, The response always
reports all configs as synced even though some are skipped; fix by tracking
actual successful inserts (e.g., introduce an insertedCount or use
skippedConfigs.length) during the loop that uses skippedConfigs and stmt.run
inside the transaction, then call bulkSync() and return res.json({ synced:
insertedCount }) (or res.json({ synced: configs.length - skippedConfigs.length
})) instead of always using configs.length so the client gets the accurate
number of rows written. Ensure the counter/skippedConfigs used is in the scope
where res.json is called (move declaration outside/above the transaction or
propagate the count out) and update any tests or callers expecting the previous
value.
🧹 Nitpick comments (2)
src/components/MarkdownRenderer.tsx (1)

496-510: LGTM with a small consideration on overflow in narrow containers.

The compacted error UI looks good. One minor thing: the <span> holding "Image failed" / "图片加载失败" has no truncate/min-w-0, while the surrounding flex row has no flex-wrap. In very narrow Markdown contexts (e.g., table cell or sidebar), the localized message + alt + retry button could push the row past the container width. The alt span and retry button already handle this (truncate max-w-[120px] and flex-shrink-0), so a small tweak on the message span keeps the layout clean.

♻️ Optional tweak
-        <span className="text-gray-500 dark:text-text-tertiary">
+        <span className="text-gray-500 dark:text-text-tertiary flex-shrink-0">
           {language === 'zh' ? '图片加载失败' : 'Image failed'}
         </span>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/MarkdownRenderer.tsx` around lines 496 - 510, The message span
that renders {language === 'zh' ? '图片加载失败' : 'Image failed'} can force the flex
row to overflow in narrow containers; fix it by adding truncation and flexible
width constraints (e.g., add the utility classes "truncate min-w-0" to that
span's className) so the text will ellipsize instead of pushing the row; this is
the span adjacent to the alt span (which already has "truncate max-w-[120px]")
and the Retry button (onClick={handleRetry}).
src/components/RepositoryList.tsx (1)

268-283: Consider centralizing AI-config preflight validation

This validation/message pattern is duplicated (Line 280 here, plus src/components/RepositoryCard.tsx Line 282, src/components/SubscriptionRepoCard.tsx Line 212, and src/components/DiscoveryView.tsx Line 667). A shared helper would reduce drift and keep future rule updates synchronized.

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

In `@src/components/RepositoryList.tsx` around lines 268 - 283, Extract the
duplicated preflight checks into a shared helper (e.g., ensureAIConfigReady or
validateAIConfig) that accepts parameters needed (githubToken, language,
aiConfigs, activeAIConfig) and returns a boolean or throws an error/message
string; replace the inline checks in handleAIAnalyze (in RepositoryList.tsx) and
the similar blocks in RepositoryCard.tsx, SubscriptionRepoCard.tsx, and
DiscoveryView.tsx with a single call to this helper and handle the returned
result by showing the same alert messages; ensure the helper encapsulates the
three checks (missing githubToken, missing activeConfig, and apiKeyStatus being
'decrypt_failed' or 'empty') and preserves the language-specific alert text so
all callers use the centralized logic.
🤖 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/components/settings/AIConfigPanel.tsx`:
- Line 5: The import line in AIConfigPanel imports an unused symbol
ConnectionTestResult causing a lint error; update the import statement that
currently references AIService and ConnectionTestResult so it only imports
AIService (remove ConnectionTestResult) in the AIConfigPanel.tsx file; ensure no
other references to ConnectionTestResult remain in the component and run the
linter to confirm the unused-import error is resolved.

---

Outside diff comments:
In `@server/src/routes/configs.ts`:
- Around line 141-176: The response always reports all configs as synced even
though some are skipped; fix by tracking actual successful inserts (e.g.,
introduce an insertedCount or use skippedConfigs.length) during the loop that
uses skippedConfigs and stmt.run inside the transaction, then call bulkSync()
and return res.json({ synced: insertedCount }) (or res.json({ synced:
configs.length - skippedConfigs.length })) instead of always using
configs.length so the client gets the accurate number of rows written. Ensure
the counter/skippedConfigs used is in the scope where res.json is called (move
declaration outside/above the transaction or propagate the count out) and update
any tests or callers expecting the previous value.

---

Nitpick comments:
In `@src/components/MarkdownRenderer.tsx`:
- Around line 496-510: The message span that renders {language === 'zh' ?
'图片加载失败' : 'Image failed'} can force the flex row to overflow in narrow
containers; fix it by adding truncation and flexible width constraints (e.g.,
add the utility classes "truncate min-w-0" to that span's className) so the text
will ellipsize instead of pushing the row; this is the span adjacent to the alt
span (which already has "truncate max-w-[120px]") and the Retry button
(onClick={handleRetry}).

In `@src/components/RepositoryList.tsx`:
- Around line 268-283: Extract the duplicated preflight checks into a shared
helper (e.g., ensureAIConfigReady or validateAIConfig) that accepts parameters
needed (githubToken, language, aiConfigs, activeAIConfig) and returns a boolean
or throws an error/message string; replace the inline checks in handleAIAnalyze
(in RepositoryList.tsx) and the similar blocks in RepositoryCard.tsx,
SubscriptionRepoCard.tsx, and DiscoveryView.tsx with a single call to this
helper and handle the returned result by showing the same alert messages; ensure
the helper encapsulates the three checks (missing githubToken, missing
activeConfig, and apiKeyStatus being 'decrypt_failed' or 'empty') and preserves
the language-specific alert text so all callers use the centralized logic.
🪄 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

Run ID: 0f3db16a-7435-4a38-b575-d09aec712f52

📥 Commits

Reviewing files that changed from the base of the PR and between 1605c4e and a81d02d.

📒 Files selected for processing (9)
  • server/src/routes/configs.ts
  • src/components/DiscoveryView.tsx
  • src/components/MarkdownRenderer.tsx
  • src/components/ReadmeModal.tsx
  • src/components/RepositoryCard.tsx
  • src/components/RepositoryList.tsx
  • src/components/SubscriptionRepoCard.tsx
  • src/components/settings/AIConfigPanel.tsx
  • src/services/aiService.ts

Comment thread src/components/settings/AIConfigPanel.tsx Outdated
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/components/settings/AIConfigPanel.tsx (1)

193-205: Same fallback applies to handleTestForm.

Same concern as handleTest: when result.success is false, alert(result.message) will surface undefined/empty if the service ever omits a message. Apply the same guarded fallback here for consistency.

♻️ Proposed refactor
       if (result.success) {
         alert(t('✅ AI服务连接成功!', '✅ AI service connection successful!'));
       } else {
-        alert(result.message);
+        alert(
+          result.message ||
+            t('AI服务测试失败,请检查网络连接和配置。', 'AI service test failed. Please check network connection and configuration.')
+        );
       }

Side note: handleTest (line 158) uses a plain success string while this one uses a -prefixed variant. If that asymmetry is unintended, consider unifying them.

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

In `@src/components/settings/AIConfigPanel.tsx` around lines 193 - 205,
handleTestForm currently alerts result.message directly when
aiService.testConnection returns success=false, which can surface
undefined/empty text; update the handler (handleTestForm) to use a guarded
fallback when showing the failure alert (e.g., alert(result.message ||
t('AI服务测试失败,请检查网络连接和配置。','AI service test failed. Please check network
connection and configuration.'))), and ensure you still clear testing state via
setTestingForm(false) in the finally block; also consider unifying the success
message formatting between handleTest and handleTestForm (remove or add the "✅"
prefix) for consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/components/settings/AIConfigPanel.tsx`:
- Around line 193-205: handleTestForm currently alerts result.message directly
when aiService.testConnection returns success=false, which can surface
undefined/empty text; update the handler (handleTestForm) to use a guarded
fallback when showing the failure alert (e.g., alert(result.message ||
t('AI服务测试失败,请检查网络连接和配置。','AI service test failed. Please check network
connection and configuration.'))), and ensure you still clear testing state via
setTestingForm(false) in the finally block; also consider unifying the success
message formatting between handleTest and handleTestForm (remove or add the "✅"
prefix) for consistency.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ba1c0ab2-31da-4787-9065-84e94a007793

📥 Commits

Reviewing files that changed from the base of the PR and between a81d02d and 37cb28c.

📒 Files selected for processing (1)
  • src/components/settings/AIConfigPanel.tsx

@AmintaCCCP AmintaCCCP merged commit cd496fb into AmintaCCCP:main Apr 27, 2026
5 checks passed
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