Skip to content

Feat/merge layered error display#23

Closed
Lin-wool wants to merge 4 commits into
TuYv:masterfrom
Lin-wool:feat/merge-layered-error-display
Closed

Feat/merge layered error display#23
Lin-wool wants to merge 4 commits into
TuYv:masterfrom
Lin-wool:feat/merge-layered-error-display

Conversation

@Lin-wool

@Lin-wool Lin-wool commented Apr 18, 2026

Copy link
Copy Markdown
Collaborator

User description

概要

feat(webview): 实现分层错误显示功能

  • 新增 ErrorBanner 组件用于显示分层错误
  • 添加 OkHttpSseClient 支持 SSE 错误流式传输
  • 更新 BridgeHandler 和 useBridge 桥接错误数据
  • 新增 bridge 类型定义
  • 添加相关设计文档和规格说明

变更文件

文件 变更说明

测试计划

  • [ ]
  • [ ]

关联 Issue

Closes #

Checklist

  • PR 标题遵循 Conventional Commits(feat:fix:docs: 等),且 type 选择正确
  • ./gradlew test 本地通过
  • 涉及 webview:cd webview && npm run build 无报错
  • 无遗留 debug 日志和注释掉的代码

PR Type

Enhancement


Description

  • Implement layered error display with typed error classification (auth/quota/temp/generic)

  • Add OkHttpSseClient error classification for API error differentiation

  • Update BridgeHandler and frontend to support typed error with actionable buttons

  • Add ErrorBanner component with type-specific UI and action handling


Diagram Walkthrough

flowchart LR
  A[OkHttpSseClient] -->|classifyErrorType| B(ClassifiedError: type + message)
  B --> C[ChatService]
  C -->|notifyError| D[BridgeHandler]
  D -->|pushJS| E[Frontend useBridge]
  E -->|onError type + message| F[ErrorBanner]
  F -->|action| G[auth: openSettings / temp: retry]
Loading

File Walkthrough

Relevant files
Enhancement
6 files
ChatService.kt
Update error handling to use typed ClassifiedError             
+13/-22 
OkHttpSseClient.kt
Add ClassifiedError and error type classification               
+25/-3   
BridgeHandler.kt
Update notifyError to include error type parameter             
+10/-6   
App.tsx
Split error state and add error action handling                   
+42/-9   
ErrorBanner.tsx
Redesign ErrorBanner with typed error display                       
+45/-22 
useBridge.ts
Update onError callback signature for type parameter         
+4/-1     
Tests
1 files
OkHttpSseClientTest.kt
Update tests for ClassifiedError return type                         
+4/-3     
Documentation
3 files
bridge.d.ts
Add error type to Bridge interface definitions                     
+2/-1     
tech-spec.md
Add comprehensive technical specification document             
+776/-0 
problem-brief.md
Add problem brief for IDEA AI assistant plugin                     
+52/-0   
Configuration changes
1 files
index.html
Update GitHub links to fork repository URL                             
+395/-429
Additional files
10 files
README.md +106/-11
auto-commit-deep-dive.md +180/-0 
command-mode-technical-deep-dive.md +774/-0 
optimization-backlog.md +91/-0   
product-spec.md +194/-0 
roadmap.md +187/-0 
2026-04-16-layered-error-display.md +666/-0 
2026-04-16-layered-error-display-design.md +170/-0 
index.html +87/-89 
App.css +0/-7     

Conflicts resolved:
- .github/workflows/pr-agent.yml: use upstream (more complete CI config)
- README.md: keep local (bilingual Chinese/English docs)
- index.html: keep local (genni613 GitHub links)
- ShellPlatform.kt: use upstream (heredoc parsing for security)
- ShellPlatformTest.kt: use upstream (new heredoc tests)
- 新增 ErrorBanner 组件用于显示分层错误
- 添加 OkHttpSseClient 支持 SSE 错误流式传输
- 更新 BridgeHandler 和 useBridge 桥接错误数据
- 新增 bridge 类型定义
- 添加相关设计文档和规格说明
@github-actions

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Potential property access error

At line 247, the code accesses error.message on the ClassifiedError parameter, but the variable name shadows the type name. While ClassifiedError does have a message property, this could be confusing. More importantly, there's no fallback handling if error.message is null or empty.

onError = { error ->
    if (cont.isActive) cont.resume(Result.failure(Exception(error.message)))
}
Debug logging left in production

Line 159 contains a logger.warn statement with "[CodePlanGUI Bridge] onLoadEnd fired, isReady=true, setting up window.__bridge" which appears to be debug logging that should be removed or changed to debug level before production.

logger.warn("[CodePlanGUI Bridge] onLoadEnd fired, isReady=true, setting up window.__bridge")
Missing error classification patterns

The classifyErrorType function uses substring matching against QUOTA_PATTERNS, AUTH_PATTERNS, and BUSY_PATTERNS. These pattern sets are referenced but their definitions are not visible in the diff. If these patterns are empty or missing, all errors would default to "generic" type, breaking the layered error display feature.

fun classifyErrorType(rawMessage: String): String {
    return when {
        QUOTA_PATTERNS.any { it in rawMessage.lowercase() } -> "quota"
        AUTH_PATTERNS.any { it in rawMessage.lowercase() } -> "auth"
        BUSY_PATTERNS.any { it in rawMessage.lowercase() } -> "temp"
        else -> "generic"
    }
}

@github-actions

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Add missing pattern constants for error classification

The classifyErrorType method references undefined pattern constants (QUOTA_PATTERNS,
AUTH_PATTERNS, BUSY_PATTERNS). These need to be defined as private constants in the
file. Without them, the code will fail to compile.

src/main/kotlin/com/github/codeplangui/api/OkHttpSseClient.kt [98]

 private const val STREAM_DEBUG_MAX_LENGTH = 1200
 
+private val QUOTA_PATTERNS = listOf("quota", "rate limit", "insufficient", "exceeded")
+private val AUTH_PATTERNS = listOf("unauthorized", "invalid", "api key", "authentication", "401", "403")
+private val BUSY_PATTERNS = listOf("rate limit", "server busy", "too many requests", "503", "429")
+
Suggestion importance[1-10]: 9

__

Why: The classifyErrorType method introduced in this PR uses QUOTA_PATTERNS, AUTH_PATTERNS, and BUSY_PATTERNS but these constants are never defined in the diff. This would cause a compilation error. The suggestion correctly identifies a missing piece that breaks the new functionality.

High

@Lin-wool Lin-wool closed this Apr 21, 2026
@Lin-wool Lin-wool deleted the feat/merge-layered-error-display branch April 21, 2026 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant