-
Notifications
You must be signed in to change notification settings - Fork 404
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: support edit_file tool #4385
Conversation
…nsumi/core into feat/mcp-server-client-poc-2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (23)
packages/ai-native/src/browser/mcp/tools/editFile.ts (3)
11-34
: 为输入参数增加额外验证或约束
当前inputSchema
功能完整,但如果后续还需要校验字段类型或限制字段为空等特殊要求,可以考虑使用strict()
或增加自定义错误信息,以防用户传入多余字段或空值。
46-70
: 说明文档可适度简化
在description
中提供了详细的使用说明,但文字较多,可以考虑在确保关键信息完整的基础上进一步简化,有助于后续维护及开发者快速理解。
72-91
: 统一错误处理与日志记录
目前的handler
方法主要聚焦正确流程,若editFileHandler
出现异常则可能需要更详细的错误日志或提示,以便迅速排查问题。同时可考虑在返回数据时添加更多上下文状态信息。packages/ai-native/src/browser/mcp/base-apply.service.ts (4)
53-64
: 可考虑为状态字段添加内联注释
codeBlockMapObservable
和pendingApplyParams
等字段对流程影响较大,可在代码中增加简短注释,便于他人理解状态驱动的通用逻辑。
65-71
: 可拓展多编辑器场景
onEditorGroupClose
仅处理了当前激活预览器销毁的情况,后续若支持同时打开多个文件的差异预览,或许需要相应的扩展逻辑。
130-139
: 重渲染的场景需警惕重复申请
在reRenderPendingApply
中,如网络延迟或用户反复点击或取消,可能会反复调用渲染逻辑,可评估是否需要限制或刷新 state。
141-203
: 差异预览与最终保存流程较完善
renderApplyResult
提供了 inline diff 和交互式的编辑流程,但若用户在中途取消后需要保留部分编辑,可能要另行处理额外的状态同步或保存策略。packages/ai-native/src/browser/mcp/tools/handlers/EditFile.ts (1)
16-16
: 需要实现 TODO 注释提到的功能这里有一个未实现的 TODO 注释,建议尽快完成文件忽略功能的实现。
需要我帮助实现文件忽略功能吗?
packages/ai-native/src/common/utils.ts (1)
52-52
: 工具函数实现清晰简洁函数实现符合单一职责原则,提供了合理的默认值。建议添加 JSDoc 注释说明参数用途和返回值格式。
+/** + * 生成MCP工具的完整名称 + * @param toolName - 工具名称 + * @param serverName - 服务器名称,默认为'sumi-builtin' + * @returns 格式为 'mcp_${serverName}_${toolName}' 的完整工具名称 + */ export const getToolName = (toolName: string, serverName = 'sumi-builtin') => `mcp_${serverName}_${toolName}`;packages/ai-native/src/browser/mcp/mcp-server.feature.registry.ts (3)
19-19
: 工具组件存储使用Record类型合适使用Record类型存储工具组件映射关系清晰明确。建议添加类型注释说明组件属性的用途。
+ /** + * 存储工具名称到对应React组件的映射关系 + */ private toolComponents: Record<string, React.FC<IMCPServerToolComponentProps>> = {};
32-42
: 工具组件注册和获取方法实现完善注册和获取方法的实现简洁有效。建议添加参数验证以提高代码健壮性。
registerToolComponent( name: string, component: React.FC<IMCPServerToolComponentProps>, serverName = 'sumi-builtin', ): void { + if (!name || !component) { + throw new Error('工具名称和组件都不能为空'); + } this.toolComponents[getToolName(name, serverName)] = component; } getToolComponent(name: string): React.FC<IMCPServerToolComponentProps> | undefined { + if (!name) { + return undefined; + } return this.toolComponents[name]; }
62-63
: 错误日志记录改进合理添加控制台错误日志有助于调试,但建议使用注入的logger而不是console。
- // eslint-disable-next-line no-console - console.error('callMCPTool error:', error); + this.logger.appendLine(`callMCPTool error: ${error}`);packages/ai-native/src/browser/chat/chat.internal.service.ts (2)
60-66
: 请求处理逻辑优化合理在发送请求后触发重新生成事件的逻辑合理。建议添加错误处理。
sendRequest(request: ChatRequestModel, regenerate = false) { + if (!request) { + throw new Error('请求对象不能为空'); + } const result = this.chatManagerService.sendRequest(this.#sessionModel.sessionId, request, regenerate); if (regenerate) { this._onRegenerateRequest.fire(); } return result; }
68-71
: 取消请求处理完善取消请求后触发事件的实现简洁明了。建议添加会话ID验证。
cancelRequest() { + if (!this.#sessionModel?.sessionId) { + throw new Error('无效的会话ID'); + } this.chatManagerService.cancelRequest(this.#sessionModel.sessionId); this._onCancelRequest.fire(); }packages/ai-native/src/browser/components/ChatToolRender.tsx (1)
38-48
: 建议优化 getParsedArgs 函数的错误处理当前的错误处理比较简单,建议添加更详细的错误日志以便于调试。
const getParsedArgs = () => { try { if (value.state !== 'complete' && value.state !== 'result') { return {}; } return JSON.parse(value.function?.arguments || '{}'); } catch (error) { + console.warn('Failed to parse function arguments:', error); return {}; } };
packages/ai-native/src/browser/mcp/tools/components/EditFile.tsx (1)
25-67
: 建议重构状态渲染逻辑
renderStatus
函数包含了大量重复的 Popover 结构。建议将通用部分抽取为独立组件。+const StatusPopover = ({ status, icon, color, onClick, children }) => ( + <Popover title={status} id={`edit-file-tool-status-${status}`}> + <Icon + iconClass={icon} + color={color} + onClick={onClick} + > + {children} + </Icon> + </Popover> +); const renderStatus = (codeBlockData: CodeBlockData, onRerender: () => void, onReveal: () => void) => { switch (codeBlockData.status) { case 'generating': return <Loading />; case 'pending': - return ( - <Popover title={status} id={'edit-file-tool-status-pending'}> - <Icon iconClass='codicon codicon-circle-large' onClick={onRerender} /> - </Popover> - ); + return <StatusPopover status="pending" icon="codicon codicon-circle-large" onClick={onRerender} />; // ... 其他状态类似处理 } };packages/ai-native/src/browser/mcp/tools/handlers/ReadFile.ts (1)
150-156
: 建议优化文件结果存储逻辑当前实现仅在行范围被缩短时存储结果。建议考虑以下改进:
- 添加结果的过期机制
- 限制 Map 的大小以避免内存泄漏
private fileResultMap: Map< string, - { content: string; startLineOneIndexed: number; endLineOneIndexedInclusive: number } + { + content: string; + startLineOneIndexed: number; + endLineOneIndexedInclusive: number; + timestamp: number; + } > = new Map(); +private readonly MAX_CACHE_SIZE = 100; +private readonly CACHE_EXPIRY_MS = 5 * 60 * 1000; // 5 minutes +private cleanupExpiredResults() { + const now = Date.now(); + for (const [key, value] of this.fileResultMap) { + if (now - value.timestamp > this.CACHE_EXPIRY_MS) { + this.fileResultMap.delete(key); + } + } +} if (didShortenLineRange) { + if (this.fileResultMap.size >= this.MAX_CACHE_SIZE) { + const oldestKey = Array.from(this.fileResultMap.keys())[0]; + this.fileResultMap.delete(oldestKey); + } this.fileResultMap.set(fileParams.relativeWorkspacePath, { content: selectedContent, startLineOneIndexed: adjustedStart, endLineOneIndexedInclusive: adjustedEnd, + timestamp: Date.now(), }); + this.cleanupExpiredResults(); }packages/ai-native/src/common/index.ts (1)
182-182
: 系统提示属性的添加符合预期!为
IChatAgentMetadata
接口添加可选的systemPrompt
属性是一个很好的改进。这样可以让每个聊天代理配置自己的系统提示信息。建议为这个新属性添加 JSDoc 注释,说明其用途和预期值格式:
+ /** + * 系统提示信息,用于配置聊天代理的行为和角色 + */ systemPrompt?: string;packages/ai-native/src/browser/components/ChatMarkdown.tsx (1)
20-20
: 新增的控制属性符合预期!为
MarkdownProps
接口添加hideInsert
属性可以灵活控制插入按钮的显示。建议添加 JSDoc 注释说明该属性的用途:
+ /** + * 是否隐藏插入按钮 + */ hideInsert?: boolean;packages/ai-native/src/browser/components/ChatEditor.tsx (1)
34-34
: Props 接口的扩展符合预期!为
Props
接口添加hideInsert
属性,与ChatMarkdown
组件保持一致。建议添加 JSDoc 注释说明该属性的用途:
+ /** + * 是否隐藏插入按钮 + */ hideInsert?: boolean;packages/core-common/src/types/ai-native/index.ts (1)
172-181
: 模型配置选项的扩展非常全面!新增的属性可以更灵活地配置模型行为,但有一些改进建议:
providerOptions
使用any
类型过于宽松,建议定义具体的接口类型。- 温度和采样参数应该添加数值范围约束。
建议进行以下改进:
/** 模型提供商,如 openai, anthropic, deepseek */ - model?: string; + model?: 'openai' | 'anthropic' | 'deepseek' | string; /** 模型ID,如 gpt-4o-mini, claude-3-5-sonnet-20240620 */ modelId?: string; baseURL?: string; - temperature?: number; + /** 温度参数,控制输出的随机性,范围 0-2 */ + temperature?: number; - topP?: number; + /** 核采样参数,范围 0-1 */ + topP?: number; - topK?: number; + /** Top-K 采样参数,范围 >= 1 的整数 */ + topK?: number; - providerOptions?: any; + /** 模型提供商特定的额外选项 */ + providerOptions?: { + [key: string]: unknown; + };packages/ai-native/src/browser/chat/chat-proxy.service.ts (1)
74-75
: 系统提示配置结构清晰,建议提取到单独文件。系统提示的配置结构良好,包含了通信、工具调用、搜索、代码修改等清晰的分节。为了提高可维护性,建议将这个长字符串提取到一个单独的配置文件中。
建议创建一个新文件
system-prompt.ts
并应用以下更改:+// system-prompt.ts +export const SYSTEM_PROMPT = `You are a powerful agentic AI coding assistant...`; - systemPrompt: - this.preferenceService.get<string>( - AINativeSettingSectionsId.SystemPrompt, - "You are a powerful agentic AI coding assistant..." - ), + systemPrompt: + this.preferenceService.get<string>( + AINativeSettingSectionsId.SystemPrompt, + SYSTEM_PROMPT + ),Also applies to: 90-95
packages/ai-native/src/browser/mcp/tools/components/index.module.less (1)
24-25
: 建议移除重复的 border-radius 声明
edit-file-tool
类中有两个重复的 border-radius 声明:.edit-file-tool { - border-radius: 4px; border-radius: 2px; ... }
建议保留其中一个值,删除另一个重复声明。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
jest.setup.jsdom.js
(1 hunks)packages/ai-native/src/browser/ai-core.contribution.ts
(2 hunks)packages/ai-native/src/browser/chat/chat-agent.service.ts
(2 hunks)packages/ai-native/src/browser/chat/chat-model.ts
(0 hunks)packages/ai-native/src/browser/chat/chat-proxy.service.ts
(3 hunks)packages/ai-native/src/browser/chat/chat.internal.service.ts
(2 hunks)packages/ai-native/src/browser/components/ChatEditor.tsx
(2 hunks)packages/ai-native/src/browser/components/ChatMarkdown.tsx
(2 hunks)packages/ai-native/src/browser/components/ChatReply.tsx
(0 hunks)packages/ai-native/src/browser/components/ChatToolRender.tsx
(3 hunks)packages/ai-native/src/browser/index.ts
(2 hunks)packages/ai-native/src/browser/mcp/base-apply.service.ts
(1 hunks)packages/ai-native/src/browser/mcp/mcp-server.feature.registry.ts
(4 hunks)packages/ai-native/src/browser/mcp/tools/components/EditFile.tsx
(1 hunks)packages/ai-native/src/browser/mcp/tools/components/index.module.less
(1 hunks)packages/ai-native/src/browser/mcp/tools/editFile.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/handlers/EditFile.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/handlers/ReadFile.ts
(3 hunks)packages/ai-native/src/browser/mcp/tools/handlers/utils.ts
(1 hunks)packages/ai-native/src/browser/preferences/schema.ts
(1 hunks)packages/ai-native/src/browser/types.ts
(1 hunks)packages/ai-native/src/browser/widget/inline-stream-diff/inline-stream-diff.handler.tsx
(1 hunks)packages/ai-native/src/browser/widget/inline-stream-diff/live-preview.decoration.tsx
(1 hunks)packages/ai-native/src/common/index.ts
(1 hunks)packages/ai-native/src/common/utils.ts
(1 hunks)packages/ai-native/src/node/base-language-model.ts
(3 hunks)packages/ai-native/src/node/mcp-server-manager-impl.ts
(2 hunks)packages/ai-native/src/node/openai/openai-language-model.ts
(1 hunks)packages/core-common/src/settings/ai-native.ts
(1 hunks)packages/core-common/src/types/ai-native/index.ts
(1 hunks)packages/i18n/src/common/en-US.lang.ts
(1 hunks)packages/i18n/src/common/zh-CN.lang.ts
(1 hunks)packages/startup/entry/sample-modules/ai-native/apply.service.ts
(1 hunks)packages/startup/entry/sample-modules/index.ts
(2 hunks)tools/dev-tool/src/webpack.js
(1 hunks)
💤 Files with no reviewable changes (2)
- packages/ai-native/src/browser/chat/chat-model.ts
- packages/ai-native/src/browser/components/ChatReply.tsx
✅ Files skipped from review due to trivial changes (1)
- jest.setup.jsdom.js
🧰 Additional context used
🪛 Biome (1.9.4)
packages/ai-native/src/browser/types.ts
[error] 403-405: Shouldn't redeclare 'MCPServerContribution'. Consider to delete it or rename it.
'MCPServerContribution' is defined here:
(lint/suspicious/noRedeclare)
[error] 406-407: Shouldn't redeclare 'TokenMCPServerRegistry'. Consider to delete it or rename it.
'TokenMCPServerRegistry' is defined here:
(lint/suspicious/noRedeclare)
🪛 GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
packages/ai-native/src/browser/types.ts
[failure] 385-385:
Cannot redeclare block-scoped variable 'MCPServerContribution'.
[failure] 387-387:
Cannot redeclare block-scoped variable 'TokenMCPServerRegistry'.
🪛 GitHub Check: build (ubuntu-latest, 20.x)
packages/ai-native/src/browser/types.ts
[failure] 385-385:
Cannot redeclare block-scoped variable 'MCPServerContribution'.
[failure] 387-387:
Cannot redeclare block-scoped variable 'TokenMCPServerRegistry'.
🪛 GitHub Check: unittest (ubuntu-latest, 18.x, node)
packages/ai-native/src/browser/types.ts
[failure] 385-385:
Cannot redeclare block-scoped variable 'MCPServerContribution'.
[failure] 387-387:
Cannot redeclare block-scoped variable 'TokenMCPServerRegistry'.
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: unittest (ubuntu-latest, 18.x, node)
- GitHub Check: build (macos-latest, 20.x)
- GitHub Check: unittest (macos-latest, 18.x, jsdom)
- GitHub Check: ubuntu-latest, Node.js 20.x
- GitHub Check: build-windows
- GitHub Check: unittest (macos-latest, 18.x, node)
🔇 Additional comments (46)
packages/ai-native/src/browser/mcp/tools/editFile.ts (2)
1-3
: 导入依赖结构简洁且易维护
这些导入符合项目需求,暂无明显问题。
36-44
: 领域注解与注册逻辑设计合理
通过@Domain(MCPServerContribution)
配合registerMCPServer
,使工具在 MCP 中可扩展性更强,整体设计无明显问题。packages/startup/entry/sample-modules/ai-native/apply.service.ts (4)
1-20
: 导入依赖结构合理
该文件中导入的模块与依赖保持一致性,无明显问题。
21-22
: 类声明清晰
ApplyService
继承了BaseApplyService
,结构清楚,命名规范,没有额外问题。
23-34
: 自动注入依赖实现简洁
通过@Autowired
注入相关服务,能保持代码可读性与可维护性。
35-139
: 加强错误处理与并发场景验证
doApply
的实现逻辑较为清晰,但仍建议在接收 AI 服务响应后增加对部分失败、中途取消或合并冲突等情况的更精细处理。同时,可考虑在多文件并行编辑场景下的并发安全性。packages/ai-native/src/browser/mcp/base-apply.service.ts (13)
1-2
: 导入diff
用于生成补丁的实现合乎需求
无额外问题。
24-52
: 构造函数内事件注册清晰
针对 cancel、regenerate 请求的监听逻辑明确,有助于统一管理所有进行中的操作。
73-79
: 查询方法简单明了
getCodeBlock
对映射表的调用直观易懂,暂无问题。
81-87
: 更新逻辑简洁
updateCodeBlock
使用transaction
对标签式状态更新,较好地避免了重复渲染。
89-107
: 注册代码块并判重的思路可行
若后续需要进一步区分同路径多块需求,可在此逻辑上增加版本号或上下文信息。
109-128
: 并发冲突或异常需更精确处理
apply
方法在处理异常时仅更新状态并抛出错误,如存在重试或多线程场景,需进一步考虑并发冲突带来的状态不一致。
205-218
: 单点取消操作逻辑清晰
在此基础上,如需批量取消或回滚也已具备可拓展性。
220-226
: 全部取消场景易于理解
cancelAllApply
遍历映射表并逐一取消,整体实现简洁。
228-238
: 消息级的清理可避免残留状态
disposeApplyForMessage
能根据 messageId 定向释放资源,思路明晰。
240-258
: 定位改动范围的实现较直观
revealApplyPosition
通过解析 diff 内容完成定位,满足主要使用场景。可考虑在大文件中进一步优化搜索效率。
260-265
: 抽象方法结构良好
doApply
方法留给子类具体实现,充分满足可扩展需求。
266-275
: 生成代码块唯一 ID 并结合会话信息
generateBlockId
通过 sessionId、用户最后一次消息等要素生成唯一标识,思路合理。
278-289
: 数据结构定义精简
CodeBlockData
及CodeBlockStatus
均清晰地说明了可能状态,有助于后期维护扩展。packages/ai-native/src/browser/mcp/tools/handlers/utils.ts (1)
1-3
: 代码实现清晰简洁!函数实现了一个简单且有效的代码块ID生成机制,使用字符串模板进行拼接,保证了ID的唯一性。
packages/ai-native/src/browser/mcp/tools/handlers/EditFile.ts (1)
5-9
: 注释清晰地说明了类的用途类的注释很好地解释了其功能和职责。
packages/ai-native/src/node/openai/openai-language-model.ts (1)
22-23
: 模型标识符处理逻辑优化得当!通过添加可选的
modelId
参数提高了灵活性,同时保持了向后兼容性。默认使用 'qwen-max' 作为模型标识符是个不错的选择。packages/startup/entry/sample-modules/index.ts (1)
27-30
: 依赖注入配置正确!服务提供者的配置遵循了依赖注入模式,通过 token 和 useClass 正确地绑定了服务实现。
packages/core-common/src/settings/ai-native.ts (1)
40-44
: 枚举值定义合理且文档完善新增的系统提示配置枚举值遵循了现有的命名规范,并提供了清晰的文档注释。
packages/ai-native/src/browser/chat/chat.internal.service.ts (1)
30-34
: 事件发射器定义规范取消和重新生成请求的事件发射器定义遵循了现有模式,实现清晰。
packages/ai-native/src/browser/components/ChatToolRender.tsx (2)
17-17
: 使用 useInjectable 获取 MCP 服务器注册表通过依赖注入获取 MCP 服务器注册表实例,这是一个良好的实践。
56-86
:❓ Verification inconclusive
验证工具组件的渲染逻辑
返回数组包含主要 JSX 结构和条件渲染的工具组件是一个灵活的设计。但是需要确保:
- 工具组件是否正确处理所有可能的状态
- 是否需要为工具组件添加错误边界
🏁 Script executed:
#!/bin/bash # 搜索工具组件的错误处理相关代码 rg "try|catch|Error" --type tsxLength of output: 61
审查建议:完善工具组件错误处理逻辑
在
packages/ai-native/src/browser/components/ChatToolRender.tsx
中渲染 ToolComponent 的实现很灵活,返回主 JSX 结构与条件渲染的 ToolComponent。但目前没有发现对 ToolComponent 内部可能出现错误的捕获逻辑,也未包裹错误边界。请重点检查以下内容:
- 确认 ToolComponent 是否在内部或被使用时捕获了所有可能的运行时错误。
- 如果 ToolComponent 未实现错误处理,建议在其外层添加错误边界或在组件内部增加 try/catch 逻辑,以确保在发生错误时不会导致整个界面崩溃。
请运行上面修正后的脚本以进一步验证项目中是否存在错误处理相关的实现,然后根据验证结果进行适当调整。
🏁 Script executed:
[run_scripts] #!/bin/bash # 修正文件类型参数,搜索项目中的错误处理相关逻辑(try/catch/Error) rg "try|catch|Error" -g "*.tsx"Length of output: 67692
请验证工具组件的错误处理实现
目前在
packages/ai-native/src/browser/components/ChatToolRender.tsx
中,我们看到对获取并渲染ToolComponent
的过程中已经添加了try/catch
逻辑(例如在文件中存在try { ... } catch (error) { ... }
的实现),这表明对异常情况已做初步保护。但仍建议关注以下两点:
检查
ToolComponent
内部错误处理是否完善
确认ToolComponent
是否在其内部对所有可能出现的异常情况均进行了有效捕获,以防止未处理的错误传递导致整个渲染树崩溃。考虑是否需要额外的错误边界
如果ToolComponent
的内部错误捕获不够全面,建议在ChatToolRender
中为ToolComponent
渲染增加一个 ErrorBoundary 包裹层,以便在发生错误时能够显示友好的错误提示并保证整体 UI 的稳定性。请根据以上建议进一步验证并测试工具组件在各类异常状态下的表现,确保异常均能被妥善处理。
🧰 Tools
🪛 Biome (1.9.4)
[error] 57-58: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
packages/ai-native/src/browser/mcp/tools/components/EditFile.tsx (1)
97-99
: 验证迭代计数的边界条件当前的条件检查可能会在某些边缘情况下导致组件不渲染。建议添加日志以跟踪这些情况。
if (!args || !codeBlockData || codeBlockData.iterationCount > 1) { + console.debug('EditFileToolComponent not rendered:', { args, codeBlockData }); return null; }
packages/ai-native/src/browser/preferences/schema.ts (1)
125-129
: 系统提示配置的添加符合预期新增的系统提示配置项集成得当,使用了正确的类型和本地化描述。
packages/ai-native/src/node/mcp-server-manager-impl.ts (1)
65-67
: 工具名称生成逻辑优化使用
getToolName
工具函数替代直接字符串拼接是一个好的改进,提高了代码的可维护性和一致性。packages/ai-native/src/node/base-language-model.ts (3)
55-59
: 增强了模型配置的灵活性新增的参数允许更细粒度地控制模型行为:
- modelId:支持指定具体的模型
- temperature:控制输出的随机性
- topP 和 topK:用于控制采样策略
- providerOptions:支持提供者特定的选项
这些参数的添加提高了模型调用的可配置性。
73-73
: 改进了模型标识符的获取方法通过添加可选的
modelId
参数,使得模型标识符的获取更加灵活。
105-117
: 优化了流式请求的配置选项在
streamText
调用中合理地使用了新增的参数:
- 设置了合适的默认值(topP: 0.8, topK: 1)
- 保持了原有参数的完整性
packages/ai-native/src/browser/chat/chat-agent.service.ts (1)
123-128
: 正确实现了系统提示功能在调用代理前,检查并添加系统提示消息:
- 使用
unshift
确保系统提示位于历史记录的开始位置- 正确设置了消息角色为
System
packages/ai-native/src/browser/mcp/tools/handlers/ReadFile.ts (2)
15-18
: 文件结果映射的类型定义清晰Map 的类型定义明确指定了键值对的结构,便于类型检查和代码维护。
187-191
: 获取文件读取结果的方法实现简洁
getFileReadResult
方法的实现简单明了,接口设计合理。packages/ai-native/src/browser/index.ts (1)
60-60
: 正确集成了文件编辑工具EditFileTool 的导入和注册符合模块的现有结构:
- 导入位置正确
- 在 MCP Server Contributions 部分正确注册
Also applies to: 101-101
packages/ai-native/src/browser/components/ChatEditor.tsx (1)
105-115
: 插入按钮的条件渲染实现正确!使用
!hideInsert
条件来控制插入按钮的显示,实现逻辑清晰。packages/ai-native/src/browser/chat/chat-proxy.service.ts (1)
94-95
: 用户信息注入实现合理。通过模板字符串动态注入操作系统版本和工作区路径的实现方式简洁有效,为 AI 提供了必要的上下文信息。
tools/dev-tool/src/webpack.js (1)
214-217
: 代码格式调整提高了可读性。
- ForkTsCheckerWebpackPlugin 配置结构更清晰
- EXTENSION_WORKER_HOST 路径拼接格式更规范
Also applies to: 225-236
packages/ai-native/src/browser/types.ts (1)
363-367
: 新增工具组件属性接口定义清晰。IMCPServerToolComponentProps 接口定义了工具组件的状态、参数和结果属性,结构合理。
packages/ai-native/src/browser/widget/inline-stream-diff/inline-stream-diff.handler.tsx (1)
72-74
: URI 访问器实现简洁合理。新增的 uri getter 提供了一个简洁的方式来访问原始模型的 URI,实现恰当。
packages/ai-native/src/browser/widget/inline-stream-diff/live-preview.decoration.tsx (1)
514-520
: 代码实现清晰简洁!方法实现正确,使用了恰当的数组方法,并且有良好的 JSDoc 文档。
packages/i18n/src/common/zh-CN.lang.ts (1)
1283-1283
: 本地化文案翻译恰当!新增的系统提示词本地化条目翻译准确,并且与其他偏好设置的翻译风格保持一致。
packages/i18n/src/common/en-US.lang.ts (1)
1517-1517
: 代码变更看起来不错!新增的本地化字符串
'preference.ai.native.chat.system.prompt': 'Default Chat System Prompt'
遵循了现有的命名约定,并且描述清晰。这个变更支持了自定义系统提示的功能。packages/ai-native/src/browser/mcp/tools/components/index.module.less (1)
1-38
: 样式实现得很好!样式定义清晰,使用了适当的 CSS 变量来处理颜色和边框,布局结构合理。嵌套规则和全局选择器的使用也很恰当。
There was a problem hiding this 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
🧹 Nitpick comments (9)
packages/ai-native/src/browser/chat/chat-proxy.service.ts (1)
89-96
: 建议重构系统提示配置以提高可维护性当前的系统提示配置较长且包含了动态信息的字符串拼接,建议进行以下优化:
- 将系统提示的各个部分拆分为独立的常量
- 创建专门的方法来生成用户信息部分
建议按照以下方式重构:
+ private readonly TOOL_CALLING_PROMPT = `<tool_calling> + You have access to tools to assist with tasks. Follow these rules: + ... + </tool_calling>`; + + private readonly CODE_CHANGES_PROMPT = `<making_code_changes> + When modifying code: + ... + </making_code_changes>`; + + private getUserInfoPrompt(): string { + return `<user_info> + The user's OS version is ${this.applicationService.frontendOS}. + The absolute path of the user's workspace is ${this.appConfig.workspaceDir}. + </user_info>`; + } + metadata: { systemPrompt: this.preferenceService.get<string>( AINativeSettingSectionsId.SystemPrompt, - 'You are a powerful AI coding assistant...' + `You are a powerful AI coding assistant... + ${this.TOOL_CALLING_PROMPT} + ${this.CODE_CHANGES_PROMPT}` ) + - `\n\n<user_info>\nThe user's OS version is ${this.applicationService.frontendOS}. The absolute path of the user's workspace is ${this.appConfig.workspaceDir}.\n</user_info>`, + `\n\n${this.getUserInfoPrompt()}`, },packages/ai-native/src/browser/mcp/tools/components/EditFile.tsx (3)
1-24
: 建议优化导入语句的组织结构建议按照以下顺序组织导入语句以提高可读性:
- React 相关导入
- @opensumi 相关导入
- monaco-editor 相关导入
- 本地组件和服务导入
- 样式导入
import cls from 'classnames'; import React, { useMemo } from 'react'; import { Icon, Popover } from '@opensumi/ide-components'; import { AppConfig, LabelService, URI, Uri, detectModeId, path, useAutorun, useInjectable, } from '@opensumi/ide-core-browser'; import { Loading } from '@opensumi/ide-core-browser/lib/components/ai-native'; + import { ILanguageService } from '@opensumi/monaco-editor-core/esm/vs/editor/common/languages/language'; import { IModelService } from '@opensumi/monaco-editor-core/esm/vs/editor/common/services/model'; import { StandaloneServices } from '@opensumi/monaco-editor-core/esm/vs/editor/standalone/browser/standaloneServices'; import { ChatMarkdown } from '../../../components/ChatMarkdown'; import { IMCPServerToolComponentProps } from '../../../types'; import { BaseApplyService, CodeBlockData } from '../../base-apply.service'; import styles from './index.module.less';
26-58
: 建议增强状态处理的类型安全性当前的状态处理逻辑可以通过以下方式优化:
- 使用 TypeScript 枚举或常量定义状态类型
- 为每个状态定义统一的图标和标题配置
+const STATUS_CONFIG = { + generating: { + icon: '', + title: '生成中', + }, + pending: { + icon: 'codicon codicon-circle-large', + title: '待处理', + }, + success: { + icon: 'codicon codicon-check-all', + title: '成功', + }, + failed: { + icon: 'codicon codicon-error', + title: '失败', + color: 'var(--vscode-input-errorForeground)', + }, + cancelled: { + icon: 'codicon codicon-close', + title: '已取消', + color: 'var(--vscode-input-placeholderForeground)', + }, +} as const; + +type StatusType = keyof typeof STATUS_CONFIG;
92-116
: 建议提升组件的可访问性和代码组织当前的渲染逻辑可以通过以下方式优化:
- 添加适当的 ARIA 属性以提升可访问性
- 提取点击事件处理函数
- 优化迭代计数的显示逻辑
+ const handleHeaderClick = () => { + if (codeBlockData.status === 'pending') { + applyService.reRenderPendingApply(); + } else if (codeBlockData.status === 'success') { + applyService.revealApplyPosition(codeBlockData.id); + } + }; + return ( - <div className={styles['edit-file-tool']}> + <div className={styles['edit-file-tool']} role="region" aria-label="文件编辑工具"> <div className={cls(styles['edit-file-tool-header'], { clickable: codeBlockData.status === 'pending' || codeBlockData.status === 'success', })} - onClick={() => { - if (codeBlockData.status === 'pending') { - applyService.reRenderPendingApply(); - } else if (codeBlockData.status === 'success') { - applyService.revealApplyPosition(codeBlockData.id); - } - }} + onClick={handleHeaderClick} + role="button" + tabIndex={0} + aria-label={`${target_file} - ${codeBlockData.status}`} >packages/ai-native/src/browser/mcp/base-apply.service.ts (5)
23-51
: 建议添加类级别的文档注释建议为
BaseApplyService
类添加完整的 JSDoc 文档注释,包括:
- 类的主要职责和用途
- 各个依赖服务的作用
- 事件监听器的处理逻辑
示例:
+/** + * 基础代码应用服务,负责管理代码块的注册、应用和取消等操作。 + * + * @remarks + * 该服务通过事件总线处理取消和重新生成请求,并维护代码块的状态。 + */ export abstract class BaseApplyService extends WithEventBus {
182-184
: 需要实现自动保存功能当前有一个未实现的自动保存功能 TODO 注释。建议:
- 创建相关 issue 来追踪此功能
- 实现自动保存以提升用户体验
是否需要我帮助创建一个实现自动保存功能的 issue?
141-203
: 建议优化 renderApplyResult 方法的错误处理
renderApplyResult
方法中的错误处理可以更加完善:
- 在创建 diff previewer 时应该添加错误处理
- 在设置新内容时应该添加错误处理
建议添加以下错误处理:
const previewer = inlineDiffController.createDiffPreviewer( editor, Selection.fromRange(range, SelectionDirection.LTR), { disposeWhenEditorClosed: true, renderRemovedWidgetImmediately: true, }, -) as LiveInlineDiffPreviewer; +); +if (!(previewer instanceof LiveInlineDiffPreviewer)) { + throw new Error('Failed to create diff previewer'); +}
248-251
: 建议加强 hunk 信息解析的健壮性当前的正则表达式解析可能在某些边缘情况下失败。建议:
- 添加正则匹配失败的错误处理
- 使用更严格的正则表达式模式
建议修改为:
-const [, , , start, end] = hunkInfo.match(/@@ -(\d+),(\d+) \+(\d+),(\d+) @@/)!; +const match = hunkInfo.match(/^@@ -\d+,\d+ \+(\d+),(\d+) @@$/); +if (!match) { + throw new Error('Invalid hunk format'); +} +const [, start, end] = match;
278-288
: 建议为类型定义添加文档注释建议为
CodeBlockData
接口和CodeBlockStatus
类型添加详细的文档注释。示例:
+/** + * 代码块数据结构 + * @property id - 代码块的唯一标识符 + * @property content - 代码块的内容 + * @property relativePath - 相对于工作区的文件路径 + * @property status - 代码块的当前状态 + * @property iterationCount - 迭代次数 + * @property createdAt - 创建时间戳 + * @property applyResult - 应用结果(如果有) + */ export interface CodeBlockData { // ... } +/** + * 代码块状态枚举 + * - generating: 正在生成中 + * - pending: 等待应用 + * - success: 应用成功 + * - rejected: 被拒绝 + * - failed: 应用失败 + * - cancelled: 已取消 + */ export type CodeBlockStatus = 'generating' | 'pending' | 'success' | 'rejected' | 'failed' | 'cancelled';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/ai-native/src/browser/ai-core.contribution.ts
(1 hunks)packages/ai-native/src/browser/chat/chat-proxy.service.ts
(3 hunks)packages/ai-native/src/browser/mcp/base-apply.service.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/components/EditFile.tsx
(1 hunks)packages/ai-native/src/browser/mcp/tools/components/index.module.less
(1 hunks)packages/ai-native/src/browser/widget/inline-stream-diff/live-preview.component.tsx
(1 hunks)packages/ai-native/src/browser/widget/inline-stream-diff/live-preview.decoration.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/ai-native/src/browser/ai-core.contribution.ts
- packages/ai-native/src/browser/widget/inline-stream-diff/live-preview.decoration.tsx
- packages/ai-native/src/browser/mcp/tools/components/index.module.less
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-windows
🔇 Additional comments (3)
packages/ai-native/src/browser/widget/inline-stream-diff/live-preview.component.tsx (1)
68-71
: 新增的属性定义清晰且有意义。新增的
acceptPartialEditCount
属性与接口中其他计数属性(如totalPartialEditCount
和resolvedPartialEditCount
)保持一致的命名风格,并且通过注释清晰地说明了其用途。packages/ai-native/src/browser/chat/chat-proxy.service.ts (1)
2-2
: 依赖注入实现正确!新增的
AppConfig
依赖注入遵循了项目规范,用于获取工作区目录信息。Also applies to: 74-75
packages/ai-native/src/browser/mcp/tools/components/EditFile.tsx (1)
65-67
: 建议增强路径处理和错误处理当前的路径处理和参数解构可以更加健壮:
- 需要验证
target_file
路径的合法性- 建议添加路径规范化处理
- 应该处理
workspaceDir
不存在的情况- const { target_file = '', code_edit } = args || {}; - const absolutePath = path.join(appConfig.workspaceDir, target_file); - const codeBlockData = applyService.getCodeBlock(absolutePath); + const { target_file = '', code_edit } = args || {}; + if (!appConfig.workspaceDir) { + console.error('工作区目录未定义'); + return null; + } + const normalizedPath = path.normalize(target_file); + if (!normalizedPath || path.isAbsolute(normalizedPath)) { + console.error('无效的目标文件路径'); + return null; + } + const absolutePath = path.join(appConfig.workspaceDir, normalizedPath); + const codeBlockData = applyService.getCodeBlock(absolutePath);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
♻️ Duplicate comments (2)
packages/ai-native/src/browser/mcp/base-apply.service.ts (1)
278-279
:⚠️ Potential issue需要实现完整的资源清理逻辑。
当前的资源清理仅删除了
codeBlockMapObservable
中的条目。建议:
- 清理所有相关的事件监听器
- 取消所有进行中的操作
- 释放所有相关的资源
packages/ai-native/src/browser/types.ts (1)
387-406
:⚠️ Potential issue需要移除重复的接口声明。
以下接口存在重复声明:
MCPServerContribution
MCPLogger
MCPToolDefinition
建议删除重复的声明,保留前面的定义。
🧹 Nitpick comments (8)
packages/startup/entry/sample-modules/ai-native/apply.service.ts (1)
139-139
: 需要实现诊断信息处理TODO 注释提到需要处理诊断信息和迭代,但尚未实现。
建议添加以下功能:
- 收集和分析诊断信息
- 实现迭代修复逻辑
- 提供诊断结果的可视化展示
需要我帮您实现这些功能或创建相关 issue 吗?
packages/ai-native/src/browser/mcp/tools/components/EditFile.tsx (1)
27-59
: 建议将状态渲染逻辑抽取为独立组件renderStatus 函数包含了大量重复的 Popover 结构,建议重构为独立组件以提高可维护性。
+const StatusPopover = ({ status, iconClass, color }: StatusPopoverProps) => ( + <Popover title={status} id={`edit-file-tool-status-${status}`}> + <Icon iconClass={iconClass} color={color} /> + </Popover> +); const renderStatus = (codeBlockData: CodeBlockData) => { const status = codeBlockData.status; switch (status) { case 'generating': return <Loading />; case 'pending': - return ( - <Popover title={status} id={'edit-file-tool-status-pending'}> - <Icon iconClass='codicon codicon-circle-large' /> - </Popover> - ); + return <StatusPopover status={status} iconClass="codicon codicon-circle-large" />; // ... 其他状态类似处理 } };packages/ai-native/src/browser/mcp/base-apply.service.ts (2)
129-149
: 建议增强错误处理机制。
apply
方法的错误处理可以更加完善:
- 需要处理
doApply
可能抛出的具体错误类型- 在发生错误时记录详细的错误信息
- 考虑添加重试机制
async apply(relativePath: string, newContent: string, instructions?: string): Promise<CodeBlockData> { const blockData = this.getCodeBlock(relativePath); if (!blockData) { throw new Error('Code block not found'); } try { if (++blockData.iterationCount > 3) { throw new Error('Max iteration count exceeded'); } blockData.status = 'generating'; blockData.content = newContent; this.updateCodeBlock(blockData); const applyDiffResult = await this.doApply(relativePath, newContent, instructions); blockData.applyResult = applyDiffResult; return blockData; } catch (err) { + console.error(`Failed to apply changes to ${relativePath}:`, err); blockData.status = 'failed'; + blockData.error = err instanceof Error ? err.message : String(err); this.updateCodeBlock(blockData); throw err; } }
210-211
: 建议在onPartialEdit
回调中添加错误处理。当前的
onPartialEdit
回调没有处理可能发生的错误。建议添加 try-catch 块来捕获和处理潜在的错误。-this.inlineDiffService.onPartialEdit((event) => { +this.inlineDiffService.onPartialEdit(async (event) => { + try { // TODO 支持自动保存 if (event.totalPartialEditCount === event.resolvedPartialEditCount) { // ... existing code ... } + } catch (err) { + console.error('Error in partial edit:', err); + blockData.status = 'failed'; + this.updateCodeBlock(blockData); + deferred.reject(err); + } });packages/ai-native/src/browser/components/ChatReply.tsx (2)
152-154
: 建议使用 TypeScript 严格模式下的可选属性类型。
messageId
属性的类型定义可以更严格。建议使用 TypeScript 的可选属性语法来明确表示这是一个可选属性。-const ToolCallRender = (props: { toolCall: IChatToolContent['content']; messageId?: string }) +const ToolCallRender = (props: { toolCall: IChatToolContent['content']; messageId: string | undefined }) -const ComponentRender = (props: { component: string; value?: unknown; messageId?: string }) +const ComponentRender = (props: { component: string; value: unknown | undefined; messageId: string | undefined })Also applies to: 179-180
157-174
: 建议优化组件重渲染逻辑。
useEffect
的依赖数组中只包含了toolCall.state
,这可能会导致其他属性变化时不会触发重渲染。建议:
- 添加
messageId
到依赖数组- 使用
useMemo
优化组件渲染-useEffect(() => { +const memoizedNode = useMemo(() => { const config = chatAgentViewService.getChatComponent('toolCall'); if (config) { const { component: Component, initialProps } = config; - setNode(<Component {...initialProps} value={toolCall} messageId={messageId} />); - return; + return <Component {...initialProps} value={toolCall} messageId={messageId} />; } - setNode( - <div> - <Loading /> - <span style={{ marginLeft: 4 }}>正在加载组件</span> - </div>, - ); + return ( + <div> + <Loading /> + <span style={{ marginLeft: 4 }}>正在加载组件</span> + </div> + ); +}, [toolCall.state, messageId, chatAgentViewService]); +useEffect(() => { + setNode(memoizedNode); const deferred = chatAgentViewService.getChatComponentDeferred('toolCall')!; deferred.promise.then(({ component: Component, initialProps }) => { setNode(<Component {...initialProps} value={toolCall} messageId={messageId} />); }); -}, [toolCall.state]); +}, [memoizedNode]);packages/ai-native/src/browser/types.ts (1)
363-370
: 建议完善接口文档。
IMCPServerToolComponentProps
接口缺少详细的文档注释。建议添加:
- 接口的整体用途说明
- 每个属性的详细描述
- 属性值的可能取值说明
+/** + * MCP 服务器工具组件的属性接口 + */ export interface IMCPServerToolComponentProps { + /** 工具的当前状态 */ state?: 'streaming-start' | 'streaming' | 'complete' | 'result'; + /** 工具的输入参数 */ args?: Record<string, any>; + /** 工具的执行结果 */ result?: any; + /** 工具在列表中的索引 */ index?: number; + /** 关联的消息ID */ messageId?: string; + /** 工具调用的唯一标识 */ toolCallId?: string; }packages/ai-native/src/browser/mcp/tools/components/index.module.less (1)
38-40
: 建议将全局样式移至专门的样式文件。全局样式
.clickable
应该放在一个共享的样式文件中,而不是在组件特定的样式文件中定义。这样可以:
- 提高样式的可重用性
- 避免样式冲突
- 更好地组织代码
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/ai-native/src/browser/components/ChatReply.tsx
(3 hunks)packages/ai-native/src/browser/components/ChatToolRender.tsx
(3 hunks)packages/ai-native/src/browser/mcp/base-apply.service.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/components/EditFile.tsx
(1 hunks)packages/ai-native/src/browser/mcp/tools/components/index.module.less
(1 hunks)packages/ai-native/src/browser/mcp/tools/editFile.ts
(1 hunks)packages/ai-native/src/browser/types.ts
(1 hunks)packages/startup/entry/sample-modules/ai-native/apply.service.ts
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/ai-native/src/browser/mcp/tools/components/EditFile.tsx
[error] 102-104: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
- GitHub Check: build (ubuntu-latest, 20.x)
- GitHub Check: unittest (ubuntu-latest, 18.x, node)
- GitHub Check: build-windows
- GitHub Check: unittest (macos-latest, 18.x, jsdom)
- GitHub Check: ubuntu-latest, Node.js 20.x
- GitHub Check: build (macos-latest, 20.x)
- GitHub Check: unittest (macos-latest, 18.x, node)
🔇 Additional comments (1)
packages/ai-native/src/browser/mcp/tools/components/index.module.less (1)
61-68
: 建议增加颜色对比度检查。错误和警告文本的颜色使用了 VS Code 的调试控制台颜色变量,需要确保这些颜色与背景色的对比度符合 WCAG 标准。
#!/usr/bin/env python3 import colorsys import sys from wcag_contrast_ratio import rgb def get_contrast_ratio(fg_color, bg_color): return rgb(fg_color).contrast(rgb(bg_color)) # 示例检查代码 fg_colors = { 'error': '#ff0000', # 需要替换为实际的错误颜色 'warning': '#ff8800' # 需要替换为实际的警告颜色 } bg_color = '#ffffff' # 需要替换为实际的背景色 for name, color in fg_colors.items(): ratio = get_contrast_ratio(color, bg_color) print(f"{name} contrast ratio: {ratio}") if ratio < 4.5: # WCAG AA 标准 print(f"Warning: {name} color does not meet WCAG AA standards")
packages/startup/entry/sample-modules/ai-native/apply.service.ts
Outdated
Show resolved
Hide resolved
packages/ai-native/src/browser/mcp/tools/components/EditFile.tsx
Outdated
Show resolved
Hide resolved
packages/ai-native/src/browser/mcp/tools/components/index.module.less
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/core-common/src/types/ai-native/index.ts (1)
172-180
: 代码变更符合预期且文档完善新增的配置项扩展了 AI 服务的灵活性,使其能够支持多种模型提供商和更细粒度的模型参数控制。JSDoc 注释清晰地说明了每个字段的用途。
不过有以下几点建议:
providerOptions
的类型定义过于宽松,建议限制类型范围- 建议为数值类型的参数添加范围约束说明
建议按如下方式优化类型定义:
/** 模型提供商,如 openai, anthropic, deepseek */ model?: string; /** 模型ID,如 gpt-4o-mini, claude-3-5-sonnet-20240620 */ modelId?: string; baseURL?: string; + /** 采样温度,取值范围 0-1 */ temperature?: number; + /** Top-p 采样参数,取值范围 0-1 */ topP?: number; + /** Top-k 采样参数,取值范围 >= 1 的整数 */ topK?: number; - providerOptions?: any; + /** 模型提供商特定的额外配置项 */ + providerOptions?: Record<string, unknown>;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/core-common/src/types/ai-native/index.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: ubuntu-latest, Node.js 20.x
- GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
- GitHub Check: unittest (ubuntu-latest, 18.x, node)
- GitHub Check: build (ubuntu-latest, 20.x)
- GitHub Check: build-windows
- GitHub Check: build (macos-latest, 20.x)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (4)
packages/ai-native/src/browser/mcp/tools/components/EditFile.tsx (4)
1-26
: 建议优化导入语句的组织结构建议将导入语句按照以下顺序分组:
- 第三方库导入(React, classnames)
- @opensumi 核心导入
- Monaco 编辑器导入
- 本地组件和服务导入
- 样式导入
import cls from 'classnames'; import React, { useEffect, useMemo } from 'react'; +// OpenSumi core imports import { Icon, Popover } from '@opensumi/ide-components'; import { AppConfig, LabelService, MarkerSeverity, URI, Uri, detectModeId, path, useAutorun, useInjectable, } from '@opensumi/ide-core-browser'; import { Loading } from '@opensumi/ide-core-browser/lib/components/ai-native'; + +// Monaco editor imports import { ILanguageService } from '@opensumi/monaco-editor-core/esm/vs/editor/common/languages/language'; import { IModelService } from '@opensumi/monaco-editor-core/esm/vs/editor/common/services/model'; import { StandaloneServices } from '@opensumi/monaco-editor-core/esm/vs/editor/standalone/browser/standaloneServices'; +// Local imports import { ChatMarkdown } from '../../../components/ChatMarkdown'; import { IMCPServerToolComponentProps } from '../../../types'; import { BaseApplyService, CodeBlockData } from '../../base-apply.service'; +// Styles import styles from './index.module.less';
27-59
: 建议增强类型安全性建议为状态字符串定义枚举类型,并使用常量来管理状态相关的配置。这样可以:
- 提高代码的可维护性
- 避免拼写错误
- 便于状态的统一管理
enum CodeBlockStatus { GENERATING = 'generating', PENDING = 'pending', SUCCESS = 'success', FAILED = 'failed', CANCELLED = 'cancelled' } interface StatusConfig { title: string; icon: string; color?: string; } const STATUS_CONFIG: Record<CodeBlockStatus, StatusConfig> = { [CodeBlockStatus.GENERATING]: { title: 'generating', icon: '' }, [CodeBlockStatus.PENDING]: { title: 'pending', icon: 'codicon-circle-large' }, [CodeBlockStatus.SUCCESS]: { title: 'success', icon: 'codicon-check-all' }, [CodeBlockStatus.FAILED]: { title: 'failed', icon: 'codicon-error', color: 'var(--vscode-input-errorForeground)' }, [CodeBlockStatus.CANCELLED]: { title: 'cancelled', icon: 'codicon-close', color: 'var(--vscode-input-placeholderForeground)' } };
115-117
: 建议将迭代次数限制提取为配置项当前迭代次数限制(3)是硬编码的,建议将其提取为可配置项,以提高代码的可维护性和灵活性。
+const MAX_ITERATION_COUNT = 3; + export const EditFileToolComponent = (props: IMCPServerToolComponentProps) => { // ... {codeBlockData.iterationCount > 1 && ( <span className={styles['edit-file-tool-iteration-count']}> - {codeBlockData.iterationCount}/3 + {codeBlockData.iterationCount}/{MAX_ITERATION_COUNT} </span> )}
61-140
: 建议将组件拆分为更小的子组件当前组件的职责较多,建议将其拆分为更小的子组件,以提高代码的可维护性和可测试性:
- 文件头部组件(包含文件图标、名称和迭代计数)
- 状态显示组件
- 诊断信息组件
需要我为您生成拆分后的组件代码吗?
🧰 Tools
🪛 Biome (1.9.4)
[error] 100-102: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/ai-native/src/browser/mcp/base-apply.service.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/components/EditFile.tsx
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/ai-native/src/browser/mcp/tools/components/EditFile.tsx
[error] 100-102: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
- GitHub Check: unittest (ubuntu-latest, 18.x, node)
- GitHub Check: unittest (macos-latest, 18.x, jsdom)
- GitHub Check: unittest (macos-latest, 18.x, node)
- GitHub Check: ubuntu-latest, Node.js 20.x
- GitHub Check: build (ubuntu-latest, 20.x)
- GitHub Check: build (macos-latest, 20.x)
- GitHub Check: build-windows
🔇 Additional comments (3)
packages/ai-native/src/browser/mcp/tools/components/EditFile.tsx (1)
99-100
: 需要为数组元素添加 key 属性返回的数组元素缺少唯一的 key 属性,这可能导致 React 渲染性能问题。
return [ - <div className={styles['edit-file-tool']}> + <div key="edit-file-tool" className={styles['edit-file-tool']}>packages/ai-native/src/browser/mcp/base-apply.service.ts (2)
44-54
: 需要优化事件监听器的生命周期管理事件监听器的注册和清理需要更严谨的处理。建议:
- 将事件监听器的取消订阅函数收集到
disposables
集合中- 在组件销毁时确保清理这些监听器
constructor() { super(); - this.chatInternalService.onCancelRequest(() => { + const disposables = new DisposableCollection(); + disposables.push(this.chatInternalService.onCancelRequest(() => { this.cancelAllApply(); - }); - this.chatInternalService.onRegenerateRequest(() => { + })); + disposables.push(this.chatInternalService.onRegenerateRequest(() => { const messages = this.chatInternalService.sessionModel.history.getMessages(); const messageId = messages[messages.length - 1].id; messageId && this.disposeApplyForMessage(messageId); - }); + })); + this.addDispose(disposables); }
272-282
: 需要完善资源清理逻辑
disposeApplyForMessage
方法中的资源清理不完整。目前仅删除了codeBlockMapObservable
中的条目,但未处理其他可能的副作用。建议:
- 清理与代码块相关的所有资源:
- 事件监听器
- 编辑器装饰器
- 内联差异预览器
- 诊断标记
- 实现完整的资源清理流程
disposeApplyForMessage(messageId: string): void { this.codeBlockMapObservable.get().forEach((blockData) => { if (blockData.id.endsWith(':' + messageId)) { if (blockData.status === 'generating') { this.cancelApply(blockData.relativePath); } - // TODO: 副作用清理 + // 清理相关资源 + if (blockData.applyResult?.diagnosticInfos) { + const uri = URI.file(path.join(this.appConfig.workspaceDir, blockData.relativePath)); + this.markerService.getManager().removeMarkers(uri); + } + if (this.activePreviewer?.getNode()?.uri.path.toString() === blockData.relativePath) { + this.activePreviewer.dispose(); + this.activePreviewer = undefined; + } this.codeBlockMapObservable.get().delete(blockData.id); } }); }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (9)
packages/ai-native/src/browser/components/ChatToolRender.tsx (1)
17-17
: 建议增加错误处理和日志记录当工具注册表中找不到对应的工具或组件时,建议添加适当的错误处理和日志记录。
const mcpServerFeatureRegistry = useInjectable<IMCPServerRegistry>(TokenMCPServerRegistry); + if (!mcpServerFeatureRegistry) { + console.warn('MCP Server Registry not found'); + return null; + } const label = mcpServerFeatureRegistry.getMCPTool(value.function.name)?.label || value.function.name; + if (!mcpServerFeatureRegistry.getMCPTool(value.function.name)) { + console.warn(`Tool not found: ${value.function.name}`); + } const ToolComponent = mcpServerFeatureRegistry.getToolComponent(value.function.name); + if (!ToolComponent) { + console.warn(`Tool component not found: ${value.function.name}`); + }Also applies to: 22-24
packages/ai-native/src/browser/mcp/mcp-server.feature.registry.ts (2)
66-68
: 增强错误日志记录。建议增加更多错误上下文信息,以便于调试和问题排查。
- // eslint-disable-next-line no-console - console.error('callMCPTool error:', error); + // eslint-disable-next-line no-console + console.error(`callMCPTool error for tool "${name}":`, error); + console.error('Arguments:', args);
19-19
: 优化工具组件类型定义。建议使用复合键来确保服务器名称和工具名称的唯一性。
- private toolComponents: Record<string, React.FC<IMCPServerToolComponentProps>> = {}; + private toolComponents: Record<`${string}:${string}`, React.FC<IMCPServerToolComponentProps>> = {};packages/ai-native/src/browser/types.ts (1)
364-371
: 建议完善 IMCPServerToolComponentProps 接口的文档注释。为了提高代码可维护性,建议为新增的接口及其属性添加详细的文档注释。
+/** + * 工具组件的属性接口 + */ export interface IMCPServerToolComponentProps { + /** 工具执行状态 */ state?: 'streaming-start' | 'streaming' | 'complete' | 'result'; + /** 工具执行参数 */ args?: Record<string, any>; + /** 工具执行结果 */ result?: any; + /** 工具在列表中的索引 */ index?: number; + /** 消息ID */ messageId?: string; + /** 工具调用ID */ toolCallId?: string; }packages/ai-native/src/browser/mcp/tools/components/EditFile.tsx (5)
10-11
: 建议统一 URI 的导入和使用方式同时导入
URI
和Uri
可能会导致混淆。建议统一使用其中一种,推荐使用URI
,因为它是 OpenSumi 中更常用的版本。- import { - URI, - Uri, - // ... other imports - } from '@opensumi/ide-core-browser'; + import { + URI, + // ... other imports + } from '@opensumi/ide-core-browser';
27-59
: 建议将状态字符串提取为常量状态字符串直接硬编码在代码中,建议将其提取为常量以提高可维护性和复用性。
+ const STATUS = { + GENERATING: 'generating', + PENDING: 'pending', + SUCCESS: 'success', + FAILED: 'failed', + CANCELLED: 'cancelled', + } as const; const renderStatus = (codeBlockData: CodeBlockData) => { const status = codeBlockData.status; switch (status) { - case 'generating': + case STATUS.GENERATING: return <Loading />; - case 'pending': + case STATUS.PENDING: // ... rest of the code
66-67
: 建议增加类型安全性
args
的解构赋值缺少类型定义,建议添加接口定义以增加类型安全性。+ interface EditFileToolArgs { + target_file: string; + code_edit: string; + } - const { target_file = '', code_edit } = args || {}; + const { target_file = '', code_edit = '' } = (args as EditFileToolArgs) || {};
77-92
: 建议优化 useMemo 钩子的使用两个
useMemo
钩子有相似的依赖数组和返回模式,可以合并为一个以提高性能。- const icon = useMemo(() => { - if (!target_file) { - return; - } - const icon = `file-icon ${labelService.getIcon(URI.file(absolutePath))}`; - return icon; - }, [target_file, absolutePath]); - const languageId = useMemo(() => { - if (!target_file) { - return; - } - const modelService = StandaloneServices.get(IModelService); - const languageService = StandaloneServices.get(ILanguageService); - const detectedModeId = detectModeId(modelService, languageService, Uri.file(absolutePath)); - return detectedModeId; - }, [target_file, absolutePath]); + const { icon, languageId } = useMemo(() => { + if (!target_file) { + return { icon: undefined, languageId: undefined }; + } + const icon = `file-icon ${labelService.getIcon(URI.file(absolutePath))}`; + const modelService = StandaloneServices.get(IModelService); + const languageService = StandaloneServices.get(ILanguageService); + const languageId = detectModeId(modelService, languageService, URI.file(absolutePath)); + return { icon, languageId }; + }, [target_file, absolutePath]);
99-142
: 建议使用 Fragment 替代数组返回使用数组返回 React 元素不是最佳实践,建议使用 Fragment 来包裹多个元素。同时,之前的评论关于缺少 key 属性的问题已经在当前代码中得到解决。
- return [ + return ( + <React.Fragment> <div className={styles['edit-file-tool']} key={`edit-file-tool-${codeBlockData.id}`}> {/* ... */} </div>, {codeBlockData.applyResult && codeBlockData.applyResult.diagnosticInfos.length > 0 && ( <div className={styles['edit-file-tool-diagnostic-errors']} key={`edit-file-tool-diagnostic-errors-${codeBlockData.id}`} > {/* ... */} </div> )} - ]; + </React.Fragment> + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/ai-native/src/browser/components/ChatToolRender.tsx
(3 hunks)packages/ai-native/src/browser/mcp/mcp-server.feature.registry.ts
(4 hunks)packages/ai-native/src/browser/mcp/tools/components/EditFile.tsx
(1 hunks)packages/ai-native/src/browser/mcp/tools/editFile.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/listDir.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/readFile.ts
(1 hunks)packages/ai-native/src/browser/types.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/ai-native/src/browser/mcp/tools/editFile.ts
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
- GitHub Check: unittest (ubuntu-latest, 18.x, node)
- GitHub Check: build-windows
- GitHub Check: build (ubuntu-latest, 20.x)
- GitHub Check: unittest (macos-latest, 18.x, jsdom)
- GitHub Check: build (macos-latest, 20.x)
- GitHub Check: ubuntu-latest, Node.js 20.x
- GitHub Check: unittest (macos-latest, 18.x, node)
🔇 Additional comments (6)
packages/ai-native/src/browser/components/ChatToolRender.tsx (3)
4-4
: 导入和组件签名的更改看起来不错!新增的导入和可选的
messageId
属性增强了组件的类型安全性和功能性。Also applies to: 9-9, 14-15
39-49
: 需要优化参数解析逻辑当前的参数解析实现仍然存在以下问题:
- TODO 注释提到流式输出中参数不完整的问题,但没有明确的解决方案
- 对于非 complete/result 状态直接返回空对象可能导致工具组件无法正确展示初始状态
86-96
: 需要添加 key 属性ToolComponent 作为数组中的元素需要添加唯一的 key 属性。
packages/ai-native/src/browser/mcp/tools/listDir.ts (1)
36-36
: 代码变更正确且清晰。添加的 label 属性能够提供更好的工具可读性和用户体验。
packages/ai-native/src/browser/mcp/tools/readFile.ts (1)
40-40
: 代码变更正确且清晰。添加的 label 属性能够提供更好的工具可读性和用户体验。
packages/ai-native/src/browser/types.ts (1)
389-409
:⚠️ Potential issue需要移除重复的类型声明。
代码中存在重复声明的接口,这会导致编译错误:
- MCPServerContribution
- MCPLogger
- MCPToolDefinition
请移除重复的声明,保留前面的定义。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 12
🔭 Outside diff range comments (1)
packages/ai-native/src/browser/types.ts (1)
374-388
: 🛠️ Refactor suggestion建议规范化接口方法的参数类型。
IMCPServerRegistry
接口的方法参数类型需要更严格的定义:
args
参数使用any
类型过于宽松serverName
参数应该在所有相关方法中保持一致export interface IMCPServerRegistry { registerMCPTool(tool: MCPToolDefinition): void; getMCPTools(): MCPToolDefinition[]; - getMCPTool(name: string): MCPToolDefinition | undefined; + getMCPTool(name: string, serverName?: string): MCPToolDefinition | undefined; - registerToolComponent(name: string, component: React.FC<IMCPServerToolComponentProps>): void; + registerToolComponent( + name: string, + component: React.FC<IMCPServerToolComponentProps>, + serverName?: string + ): void; getToolComponent(name: string): React.FC<IMCPServerToolComponentProps> | undefined; callMCPTool( name: string, - args: any, + args: Record<string, unknown>, ): Promise<{ content: { type: string; text: string }[]; isError?: boolean; }>; }
🧹 Nitpick comments (5)
packages/ai-native/src/browser/mcp/mcp-server-proxy.service.ts (1)
34-38
: 建议考虑性能优化使用
zodToJsonSchema
转换架构是正确的做法,但每次调用$getMCPTools
都进行转换可能会影响性能。建议:
- 考虑在工具注册时就完成架构转换
- 或者对转换结果进行缓存
示例优化代码:
async $getMCPTools() { - const tools = await this.mcpServerRegistry.getMCPTools().map((tool) => + const tools = await this.mcpServerRegistry.getMCPTools().map((tool) => { + // 使用 WeakMap 缓存转换结果 + if (!this.schemaCache.has(tool.inputSchema)) { + this.schemaCache.set(tool.inputSchema, zodToJsonSchema(tool.inputSchema)); + } + return { + name: tool.name, + description: tool.description, + inputSchema: this.schemaCache.get(tool.inputSchema), + providerName: 'sumi-builtin', + }; + });packages/ai-native/src/browser/mcp/mcp-server.feature.registry.ts (2)
28-30
: 建议增加服务器名称的验证。
getMCPTool
方法中的serverName
参数应该进行非空验证,以避免潜在的运行时错误。getMCPTool(name: string, serverName = 'sumi-builtin'): MCPToolDefinition | undefined { + if (!serverName?.trim()) { + return undefined; + } return this.tools.find((tool) => getToolName(tool.name, serverName) === name); }
68-69
: 建议使用结构化的错误日志。当前的错误日志格式过于简单,建议使用更结构化的格式以便于调试和监控。
- // eslint-disable-next-line no-console - console.error('callMCPTool error:', error); + this.baseLogger.error('MCPTool execution failed', { + tool: name, + error: error instanceof Error ? error.message : String(error), + stack: error instanceof Error ? error.stack : undefined + });packages/ai-native/src/browser/types.ts (1)
351-363
: 建议完善 MCPToolDefinition 接口。
MCPToolDefinition
接口的更新引入了一些改进空间:
label
属性的用途需要在注释中说明inputSchema
类型更改为ZodEffects<any>
后应该提供类型示例export interface MCPToolDefinition { name: string; + /** + * 工具在 UI 中显示的标签文本 + * @example "编辑文件" + */ label?: string; description: string; + /** + * 输入参数的 Zod 验证模式 + * @example + * z.object({ + * path: z.string(), + * content: z.string() + * }) + */ inputSchema: ZodEffects<any>; handler: ( args: any, logger: MCPLogger, ) => Promise<{ content: { type: string; text: string }[]; isError?: boolean; }>; }packages/ai-native/src/browser/mcp/tools/replaceOpenEditorFile.ts (1)
62-67
: 建议优化编辑操作的错误处理当前的编辑操作没有检查执行结果。建议添加执行结果检查以确保编辑操作成功完成。
建议按如下方式修改代码:
- editor.monacoEditor.executeEdits('mcp.tool.replace-file', [ + const editSuccess = editor.monacoEditor.executeEdits('mcp.tool.replace-file', [ { range: fullRange, text: args.text, }, ]); + if (!editSuccess) { + logger.appendLine('Failed to execute edit operation'); + return { + content: [{ type: 'text', text: 'unknown error' }], + isError: true, + }; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
packages/ai-native/src/browser/components/ChatMarkdown.tsx
(2 hunks)packages/ai-native/src/browser/mcp/base-apply.service.ts
(1 hunks)packages/ai-native/src/browser/mcp/mcp-server-proxy.service.ts
(2 hunks)packages/ai-native/src/browser/mcp/mcp-server.feature.registry.ts
(4 hunks)packages/ai-native/src/browser/mcp/tools/components/index.module.less
(1 hunks)packages/ai-native/src/browser/mcp/tools/createNewFileWithText.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/editFile.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/findFilesByNameSubstring.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/getCurrentFilePath.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/getDiagnosticsByPath.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/getFileTextByPath.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/getOpenEditorFileDiagnostics.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/getOpenEditorFileText.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/getSelectedText.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/listDir.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/readFile.ts
(2 hunks)packages/ai-native/src/browser/mcp/tools/replaceOpenEditorFile.ts
(2 hunks)packages/ai-native/src/browser/mcp/tools/replaceOpenEditorFileByDiffPreviewer.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/runTerminalCmd.ts
(1 hunks)packages/ai-native/src/browser/types.ts
(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/ai-native/src/browser/mcp/tools/runTerminalCmd.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/ai-native/src/browser/mcp/tools/listDir.ts
- packages/ai-native/src/browser/mcp/tools/readFile.ts
- packages/ai-native/src/browser/components/ChatMarkdown.tsx
- packages/ai-native/src/browser/mcp/tools/components/index.module.less
- packages/ai-native/src/browser/mcp/base-apply.service.ts
- packages/ai-native/src/browser/mcp/tools/editFile.ts
🧰 Additional context used
🪛 GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
packages/ai-native/src/browser/mcp/tools/getSelectedText.ts
[failure] 27-27:
Type 'ZodObject<{}, "strip", ZodTypeAny, {}, {}>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/getFileTextByPath.ts
[failure] 40-40:
Type 'ZodObject<{ pathInProject: ZodString; }, "strip", ZodTypeAny, { pathInProject: string; }, { pathInProject: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/findFilesByNameSubstring.ts
[failure] 40-40:
Type 'ZodObject<{ nameSubstring: ZodString; }, "strip", ZodTypeAny, { nameSubstring: string; }, { nameSubstring: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/getDiagnosticsByPath.ts
[failure] 49-49:
Type 'ZodObject<{ filePathInProject: ZodString; }, "strip", ZodTypeAny, { filePathInProject: string; }, { filePathInProject: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/replaceOpenEditorFile.ts
[failure] 33-33:
Type 'ZodObject<{ text: ZodString; }, "strip", ZodTypeAny, { text: string; }, { text: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/createNewFileWithText.ts
[failure] 40-40:
Type 'ZodObject<{ pathInProject: ZodString; text: ZodString; }, "strip", ZodTypeAny, { text: string; pathInProject: string; }, { text: string; pathInProject: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/getOpenEditorFileDiagnostics.ts
[failure] 49-49:
Type 'ZodObject<{}, "strip", ZodTypeAny, {}, {}>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/getCurrentFilePath.ts
[failure] 27-27:
Type 'ZodObject<{}, "strip", ZodTypeAny, {}, {}>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/getOpenEditorFileText.ts
[failure] 27-27:
Type 'ZodObject<{}, "strip", ZodTypeAny, {}, {}>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/replaceOpenEditorFileByDiffPreviewer.ts
[failure] 36-36:
Type 'ZodObject<{ text: ZodString; }, "strip", ZodTypeAny, { text: string; }, { text: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
🪛 GitHub Check: unittest (ubuntu-latest, 18.x, node)
packages/ai-native/src/browser/mcp/tools/getSelectedText.ts
[failure] 27-27:
Type 'ZodObject<{}, "strip", ZodTypeAny, {}, {}>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/getFileTextByPath.ts
[failure] 40-40:
Type 'ZodObject<{ pathInProject: ZodString; }, "strip", ZodTypeAny, { pathInProject: string; }, { pathInProject: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/findFilesByNameSubstring.ts
[failure] 40-40:
Type 'ZodObject<{ nameSubstring: ZodString; }, "strip", ZodTypeAny, { nameSubstring: string; }, { nameSubstring: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/getDiagnosticsByPath.ts
[failure] 49-49:
Type 'ZodObject<{ filePathInProject: ZodString; }, "strip", ZodTypeAny, { filePathInProject: string; }, { filePathInProject: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/replaceOpenEditorFile.ts
[failure] 33-33:
Type 'ZodObject<{ text: ZodString; }, "strip", ZodTypeAny, { text: string; }, { text: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/createNewFileWithText.ts
[failure] 40-40:
Type 'ZodObject<{ pathInProject: ZodString; text: ZodString; }, "strip", ZodTypeAny, { text: string; pathInProject: string; }, { text: string; pathInProject: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/getOpenEditorFileDiagnostics.ts
[failure] 49-49:
Type 'ZodObject<{}, "strip", ZodTypeAny, {}, {}>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/getCurrentFilePath.ts
[failure] 27-27:
Type 'ZodObject<{}, "strip", ZodTypeAny, {}, {}>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/getOpenEditorFileText.ts
[failure] 27-27:
Type 'ZodObject<{}, "strip", ZodTypeAny, {}, {}>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/replaceOpenEditorFileByDiffPreviewer.ts
[failure] 36-36:
Type 'ZodObject<{ text: ZodString; }, "strip", ZodTypeAny, { text: string; }, { text: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
🪛 GitHub Check: build (ubuntu-latest, 20.x)
packages/ai-native/src/browser/mcp/tools/getSelectedText.ts
[failure] 27-27:
Type 'ZodObject<{}, "strip", ZodTypeAny, {}, {}>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/getFileTextByPath.ts
[failure] 40-40:
Type 'ZodObject<{ pathInProject: ZodString; }, "strip", ZodTypeAny, { pathInProject: string; }, { pathInProject: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/findFilesByNameSubstring.ts
[failure] 40-40:
Type 'ZodObject<{ nameSubstring: ZodString; }, "strip", ZodTypeAny, { nameSubstring: string; }, { nameSubstring: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/getDiagnosticsByPath.ts
[failure] 49-49:
Type 'ZodObject<{ filePathInProject: ZodString; }, "strip", ZodTypeAny, { filePathInProject: string; }, { filePathInProject: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/replaceOpenEditorFile.ts
[failure] 33-33:
Type 'ZodObject<{ text: ZodString; }, "strip", ZodTypeAny, { text: string; }, { text: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/createNewFileWithText.ts
[failure] 40-40:
Type 'ZodObject<{ pathInProject: ZodString; text: ZodString; }, "strip", ZodTypeAny, { text: string; pathInProject: string; }, { text: string; pathInProject: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/getOpenEditorFileDiagnostics.ts
[failure] 49-49:
Type 'ZodObject<{}, "strip", ZodTypeAny, {}, {}>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/getCurrentFilePath.ts
[failure] 27-27:
Type 'ZodObject<{}, "strip", ZodTypeAny, {}, {}>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/getOpenEditorFileText.ts
[failure] 27-27:
Type 'ZodObject<{}, "strip", ZodTypeAny, {}, {}>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/replaceOpenEditorFileByDiffPreviewer.ts
[failure] 36-36:
Type 'ZodObject<{ text: ZodString; }, "strip", ZodTypeAny, { text: string; }, { text: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
🪛 GitHub Check: ubuntu-latest, Node.js 20.x
packages/ai-native/src/browser/mcp/tools/getSelectedText.ts
[failure] 27-27:
Type 'ZodObject<{}, "strip", ZodTypeAny, {}, {}>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/getFileTextByPath.ts
[failure] 40-40:
Type 'ZodObject<{ pathInProject: ZodString; }, "strip", ZodTypeAny, { pathInProject: string; }, { pathInProject: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/findFilesByNameSubstring.ts
[failure] 40-40:
Type 'ZodObject<{ nameSubstring: ZodString; }, "strip", ZodTypeAny, { nameSubstring: string; }, { nameSubstring: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/getDiagnosticsByPath.ts
[failure] 49-49:
Type 'ZodObject<{ filePathInProject: ZodString; }, "strip", ZodTypeAny, { filePathInProject: string; }, { filePathInProject: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/replaceOpenEditorFile.ts
[failure] 33-33:
Type 'ZodObject<{ text: ZodString; }, "strip", ZodTypeAny, { text: string; }, { text: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/createNewFileWithText.ts
[failure] 40-40:
Type 'ZodObject<{ pathInProject: ZodString; text: ZodString; }, "strip", ZodTypeAny, { text: string; pathInProject: string; }, { text: string; pathInProject: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/getOpenEditorFileDiagnostics.ts
[failure] 49-49:
Type 'ZodObject<{}, "strip", ZodTypeAny, {}, {}>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/getCurrentFilePath.ts
[failure] 27-27:
Type 'ZodObject<{}, "strip", ZodTypeAny, {}, {}>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/getOpenEditorFileText.ts
[failure] 27-27:
Type 'ZodObject<{}, "strip", ZodTypeAny, {}, {}>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
packages/ai-native/src/browser/mcp/tools/replaceOpenEditorFileByDiffPreviewer.ts
[failure] 36-36:
Type 'ZodObject<{ text: ZodString; }, "strip", ZodTypeAny, { text: string; }, { text: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType
🪛 GitHub Actions: E2E Test
packages/ai-native/src/browser/mcp/tools/createNewFileWithText.ts
[error] 40-40: error TS2739: Type 'ZodObject<{ pathInProject: ZodString; text: ZodString; }, "strip", ZodTypeAny, { text: string; pathInProject: string; }, { text: string; pathInProject: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType.
🪛 GitHub Actions: CI
packages/ai-native/src/browser/mcp/tools/createNewFileWithText.ts
[error] 40-40: error TS2739: Type 'ZodObject<{ pathInProject: ZodString; text: ZodString; }, "strip", ZodTypeAny, { text: string; pathInProject: string; }, { text: string; pathInProject: string; }>' is missing the following properties from type 'ZodEffects<any, any, any>': innerType, sourceType.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-windows
🔇 Additional comments (3)
packages/ai-native/src/browser/mcp/mcp-server-proxy.service.ts (1)
1-1
: 导入语句正确添加!引入
zodToJsonSchema
工具函数用于架构转换是一个好的做法,这与整体重构方向保持一致。packages/ai-native/src/browser/mcp/mcp-server.feature.registry.ts (1)
3-6
: 工具组件注册系统的基础设施已就绪。新增的
toolComponents
属性和相关导入为工具组件注册系统提供了基础设施支持。这些更改与 PR 目标中提到的"支持 edit_file 工具"相符。Also applies to: 19-19
packages/ai-native/src/browser/mcp/tools/getDiagnosticsByPath.ts (1)
72-79
: 代码实现值得表扬路径验证的实现非常严谨,通过检查相对路径是否以 '..' 开头或是否为绝对路径来确保文件位于项目范围内,这是一个很好的安全实践。
packages/ai-native/src/browser/mcp/tools/replaceOpenEditorFileByDiffPreviewer.ts
Show resolved
Hide resolved
packages/ai-native/src/browser/mcp/tools/getOpenEditorFileDiagnostics.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (4)
packages/ai-native/src/browser/mcp/base-apply.service.ts (4)
44-58
: 🛠️ Refactor suggestion建议优化事件监听器的资源管理
当前的事件监听器管理可以改进:
- 使用统一的DisposableCollection
- 确保所有监听器都被正确清理
constructor() { super(); + const disposables = new DisposableCollection(); - this.addDispose( + disposables.push( this.chatInternalService.onCancelRequest(() => { this.cancelAllApply(); }), ); - this.addDispose( + disposables.push( this.chatInternalService.onRegenerateRequest(() => { const messages = this.chatInternalService.sessionModel.history.getMessages(); const messageId = messages[messages.length - 1].id; messageId && this.disposeApplyForMessage(messageId); }), ); + this.addDispose(disposables); }
106-121
: 🛠️ Refactor suggestion建议增加输入参数验证
registerCodeBlock
方法缺少必要的参数验证:
- 需要验证路径合法性
- 需要验证内容有效性
registerCodeBlock(relativePath: string, content: string): string { + if (!relativePath?.trim()) { + throw new Error('文件路径不能为空'); + } + if (content === undefined || content === null) { + throw new Error('代码内容不能为空'); + } + // 标准化路径 + relativePath = path.normalize(relativePath); const blockId = this.generateBlockId(relativePath);
216-250
:⚠️ Potential issue需要处理事件监听器的生命周期
在
renderApplyResult
方法中,事件监听器的清理不完整:
- 监听器应在完成时自动解除
- 需要处理错误情况下的清理
- this.addDispose( + const disposable = this.inlineDiffService.onPartialEdit((event) => { if (event.totalPartialEditCount === event.resolvedPartialEditCount) { if (event.acceptPartialEditCount > 0) { blockData.status = 'success'; // ... existing code ... deferred.resolve({ diff: diffResult, diagnosticInfos, }); + disposable.dispose(); } else { blockData.status = 'cancelled'; deferred.resolve(); + disposable.dispose(); } } }), - ); + deferred.promise.catch(() => disposable.dispose());
289-307
: 🛠️ Refactor suggestion建议改进错误处理和空值检查
revealApplyPosition
方法中的非空断言操作符使用不安全:
- 需要添加适当的空值检查
- 需要完善错误处理
revealApplyPosition(blockId: string): void { const blockData = this.codeBlockMapObservable.get().get(blockId); - if (blockData) { + if (!blockData) { + console.warn(`未找到ID为${blockId}的代码块`); + return; + } + try { const hunkInfo = blockData.applyResult?.diff.split('\n').find((line) => line.startsWith('@@')); let startLine = 0; let endLine = 0; if (hunkInfo) { - const [, , , start, end] = hunkInfo.match(/@@ -(\d+),(\d+) \+(\d+),(\d+) @@/)!; + const match = hunkInfo.match(/@@ -(\d+),(\d+) \+(\d+),(\d+) @@/); + if (!match) { + throw new Error('无效的差异信息格式'); + } + const [, , , start, end] = match; startLine = parseInt(start, 10) - 1; endLine = parseInt(end, 10) - 1; } this.editorService.open(URI.file(path.join(this.appConfig.workspaceDir, blockData.relativePath))); const editor = this.editorService.currentEditor; if (editor) { editor.setSelection(new Selection(startLine, 0, endLine, 0)); } + } catch (error) { + console.error('显示应用位置时出错:', error); } }
🧹 Nitpick comments (5)
packages/ai-native/src/browser/mcp/tools/components/EditFile.tsx (3)
1-25
: 优化导入声明
useEffect
被导入但未使用,建议移除。- 同时使用
URI
和Uri
可能会造成混淆,建议统一使用其中一个。-import React, { useEffect, useMemo } from 'react'; +import React, { useMemo } from 'react'; import { AppConfig, LabelService, MarkerSeverity, - URI, Uri, detectModeId, path, useAutorun, useInjectable, } from '@opensumi/ide-core-browser';
27-59
: 建议优化状态渲染逻辑当前状态渲染逻辑存在代码重复,可以通过配置对象和映射来简化代码。
+const STATUS_CONFIG = { + pending: { icon: 'circle-large' }, + success: { icon: 'check-all' }, + failed: { icon: 'error', color: 'var(--vscode-input-errorForeground)' }, + cancelled: { icon: 'close', color: 'var(--vscode-input-placeholderForeground)' }, +}; const renderStatus = (codeBlockData: CodeBlockData) => { const status = codeBlockData.status; - switch (status) { - case 'generating': - return <Loading />; - case 'pending': - return ( - <Popover title={status} id={'edit-file-tool-status-pending'}> - <Icon iconClass='codicon codicon-circle-large' /> - </Popover> - ); - // ... other cases - default: - return null; - } + if (status === 'generating') return <Loading />; + + const config = STATUS_CONFIG[status]; + if (!config) return null; + + return ( + <Popover title={status} id={`edit-file-tool-status-${status}`}> + <Icon + iconClass={`codicon codicon-${config.icon}`} + color={config.color} + /> + </Popover> + ); };
77-93
: 优化服务获取逻辑Monaco 服务的获取可以在组件顶层进行,避免在 useMemo 中重复获取。
export const EditFileToolComponent = (props: IMCPServerToolComponentProps) => { const labelService = useInjectable(LabelService); const appConfig = useInjectable<AppConfig>(AppConfig); const applyService = useInjectable<BaseApplyService>(BaseApplyService); + const modelService = StandaloneServices.get(IModelService); + const languageService = StandaloneServices.get(ILanguageService); // ... other code ... const languageId = useMemo(() => { if (!target_file) { return; } - const modelService = StandaloneServices.get(IModelService); - const languageService = StandaloneServices.get(ILanguageService); const detectedModeId = detectModeId(modelService, languageService, Uri.file(absolutePath)); return detectedModeId; }, [target_file, absolutePath]);packages/startup/entry/sample-modules/ai-native/apply.service.ts (2)
55-65
: 建议优化AI提示结构当前的提示文本较为简单,建议:
- 添加更详细的代码合并规则
- 明确说明如何处理冲突
- 提供具体的输出格式示例
- `Merge all changes from the <update> snippet into the <code> below. - - Preserve the code's structure, order, comments, and indentation exactly. - - Output only the updated code, enclosed within <updated-code> and </updated-code> tags. - - Do not include any additional text, explanations, placeholders, ellipses, or code fences. + `作为代码合并专家,请将<update>中的代码变更合并到<code>中: + + 合并规则: + 1. 保持原有代码结构、顺序、注释和缩进格式 + 2. 解决冲突时优先保持语义一致性 + 3. 确保合并后的代码符合语法规范 + + 输出要求: + 1. 只输出更新后的完整代码 + 2. 使用<updated-code>和</updated-code>标签包裹 + 3. 不要包含任何其他文本、说明或代码围栏 + + 示例输出格式: + <updated-code> + // 你的代码 + </updated-code>
130-130
: 需要实现诊断信息和迭代功能TODO注释提到需要添加诊断信息和迭代功能,这是重要的特性。
是否需要我帮助实现这些功能?我可以提供:
- 代码诊断信息收集的实现
- 迭代优化的逻辑设计
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (7)
packages/ai-native/__test__/browser/chat/chat-agent.service.test.ts
(1 hunks)packages/ai-native/package.json
(0 hunks)packages/ai-native/src/browser/mcp/base-apply.service.ts
(1 hunks)packages/ai-native/src/browser/mcp/tools/components/EditFile.tsx
(1 hunks)packages/ai-native/src/browser/mcp/tools/components/index.module.less
(1 hunks)packages/ai-native/src/browser/types.ts
(3 hunks)packages/startup/entry/sample-modules/ai-native/apply.service.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- packages/ai-native/package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/ai-native/src/browser/types.ts
- packages/ai-native/src/browser/mcp/tools/components/index.module.less
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
- GitHub Check: unittest (ubuntu-latest, 18.x, node)
- GitHub Check: build (ubuntu-latest, 20.x)
- GitHub Check: unittest (macos-latest, 18.x, jsdom)
- GitHub Check: build-windows
- GitHub Check: unittest (macos-latest, 18.x, node)
- GitHub Check: build (macos-latest, 20.x)
- GitHub Check: ubuntu-latest, Node.js 20.x
🔇 Additional comments (2)
packages/ai-native/src/browser/mcp/tools/components/EditFile.tsx (1)
99-143
: 优化渲染逻辑和事件处理
- 数组元素需要添加 key 属性(重复之前的评论)
- 点击处理函数可以提取出来以提高可读性
+ const handleHeaderClick = () => { + if (codeBlockData.status === 'pending') { + applyService.reRenderPendingApply(); + } else if (codeBlockData.status === 'success') { + applyService.revealApplyPosition(codeBlockData.id); + } + }; return [ - instructions && <p>{instructions}</p>, + instructions && <p key="instructions">{instructions}</p>, <div className={styles['edit-file-tool']} key={`edit-file-tool-${codeBlockData.id}`}> <div className={cls(styles['edit-file-tool-header'], { clickable: codeBlockData.status === 'pending' || codeBlockData.status === 'success', })} - onClick={() => { - if (codeBlockData.status === 'pending') { - applyService.reRenderPendingApply(); - } else if (codeBlockData.status === 'success') { - applyService.revealApplyPosition(codeBlockData.id); - } - }} + onClick={handleHeaderClick} >packages/ai-native/__test__/browser/chat/chat-agent.service.test.ts (1)
59-61
: 测试用例中正确添加了系统提示配置测试代码中为agent添加了metadata.systemPrompt属性,这与新增的系统提示功能相符。
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4385 +/- ##
==========================================
- Coverage 53.85% 53.73% -0.12%
==========================================
Files 1651 1655 +4
Lines 101670 101889 +219
Branches 21992 22036 +44
==========================================
- Hits 54754 54751 -3
- Misses 39015 39203 +188
- Partials 7901 7935 +34
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Types
Background or solution
implement #4369
Changelog
feat: support edit_file tool
Summary by CodeRabbit
新功能
EditFileTool
组件,支持文件编辑操作。ChatInternalService
的事件驱动能力,支持更多操作通知。acceptPartialEditCount
属性,提升部分编辑事件的详细信息。label
属性,增强用户友好性。重构