Skip to content
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: add search file & content tool #4396

Merged
merged 9 commits into from
Feb 21, 2025
Merged

feat: add search file & content tool #4396

merged 9 commits into from
Feb 21, 2025

Conversation

ensorrow
Copy link
Contributor

@ensorrow ensorrow commented Feb 21, 2025

Types

  • 🎉 New Features

Background or solution

feat: add search file & content tool

image

Changelog

feat: add search file & content tool

Summary by CodeRabbit

  • 新功能

    • 引入全新文件搜索及正则匹配搜索功能,提升了检索效率和结果展示。
    • 推出交互式搜索结果面板,支持文件列表的展开/收起和快速预览。
    • 增强了消息历史管理功能,支持附加消息数据的管理和事件通知。
    • 更新了工具调用方法,增加了工具调用标识,提升了调用的跟踪能力。
  • 样式

    • 优化了聊天工具及搜索组件的界面设计,改善了用户交互体验。
  • 优化

    • 调整工具调用流程并精简内部交互,增强了系统的稳定性与可维护性。

@opensumi opensumi bot added the 🎨 feature feature required label Feb 21, 2025
Copy link
Contributor

coderabbitai bot commented Feb 21, 2025

Walkthrough

本次变更主要更新了 MCP 服务器相关的接口与调用方式,对原有的 callTool 方法签名进行了修改,新增了 toolCallId 参数,并调整了所有相关测试和调用逻辑。同时,前端组件和样式文件也进行了更新,新增加了文件搜索及 Grep 搜索的工具实现,并移除了旧的文件操作工具。此外,还扩展了消息历史管理功能,允许存储和传递附加数据,以及更新了搜索服务的输入配置参数。

Changes

文件 变更摘要
packages/ai-native/__test__/node/mcp-server.test.ts
packages/ai-native/src/node/mcp-server.ts
packages/ai-native/src/node/mcp/sumi-mcp-server.ts
packages/ai-native/src/common/mcp-server-manager.ts
packages/ai-native/src/node/mcp-server-manager-impl.ts
修改 callTool 方法签名,新增 toolCallId 参数,并调整调用逻辑
packages/ai-native/src/browser/mcp/mcp-server.feature.registry.ts
packages/ai-native/src/common/tool-invocation-registry.ts
packages/ai-native/src/node/base-language-model.ts
更新工具注册与调用逻辑,扩展 handler 接口支持可选 options 参数及传递 toolCallId
packages/ai-native/src/browser/components/ChatToolRender.tsx
packages/ai-native/src/browser/mcp/tools/components/SearchResult.tsx
packages/ai-native/src/browser/mcp/tools/components/index.module.less
packages/ai-native/src/browser/mcp/tools/fileSearch.ts
packages/ai-native/src/browser/mcp/tools/grepSearch.ts
新增和优化前端组件及样式,增强文件搜索及 Grep 搜索功能的实现
删除文件:
packages/ai-native/src/browser/mcp/tools/findFilesByNameSubstring.ts
packages/ai-native/src/browser/mcp/tools/getCurrentFilePath.ts
packages/ai-native/src/browser/mcp/tools/getFileTextByPath.ts
packages/ai-native/src/browser/mcp/tools/getOpenEditorFileText.ts
packages/ai-native/src/browser/mcp/tools/getSelectedText.ts
packages/ai-native/src/browser/mcp/tools/replaceOpenEditorFile.ts
packages/ai-native/src/browser/mcp/tools/replaceOpenEditorFileByDiffPreviewer.ts
移除旧的文件操作及编辑工具实现
packages/ai-native/src/browser/mcp/tools/listDir.ts
packages/ai-native/src/browser/mcp/tools/readFile.ts
修改输入校验 schema,将 explanation 字段设为可选
packages/ai-native/src/browser/model/msg-history-manager.ts 增加 messageAdditionalMap 属性、事件 emitter 以及 setMessageAdditionalgetMessageAdditional 方法,实现消息附加数据管理
packages/core-common/src/types/ai-native/index.ts IHistoryChatMessage 接口新增可选属性 additional,用于存储附加数据
packages/search/src/browser/search.service.ts
packages/search/src/common/content-search.ts
更新 doSearch 方法签名支持 includeexcludemaxResults 参数,并调整搜索结果的文档说明
packages/ai-native/src/browser/index.ts 更新导入与 providers 数组,替换部分旧工具为 FileSearchToolGrepSearchTool

Sequence Diagram(s)

sequenceDiagram
    participant C as 客户端
    participant S as MCPServerImpl
    participant R as 工具调用注册表
    participant H as 工具处理器
    C->>S: callTool(toolName, toolCallId, argString)
    S->>R: 转换请求,传递包含 toolCallId 的选项
    R->>H: 执行 handler(argString, options)
    H-->>R: 返回处理结果
    R-->>S: 返回处理后的结果
    S-->>C: 返回最终结果
Loading

Possibly related PRs

Suggested reviewers

  • Ricbet
  • hacke2

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

yarn install v1.22.22
[1/4] Resolving packages...
warning [email protected]: This version is no longer supported. Please see https://eslint.org/version-support for other options.
warning eslint > @humanwhocodes/[email protected]: Use @eslint/config-array instead
warning eslint > @humanwhocodes/config-array > @humanwhocodes/[email protected]: Use @eslint/object-schema instead
warning eslint > file-entry-cache > flat-cache > [email protected]: Rimraf versions prior to v4 are no longer supported
warning eslint > file-entry-cache > flat-cache > rimraf > [email protected]: Glob versions prior to v9 are no longer supported
warning eslint > file-entry-cache > flat-cache > rimraf > glob > [email protected]: This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful.
warning eslint-import-resolver-typescript > [email protected]: Glob versions prior to v9 are no longer supported
error Couldn't find any versions for "@opensumi/ide-dev-tool" that matches "workspace:*"
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4e9c60d and 325fb67.

📒 Files selected for processing (5)
  • packages/ai-native/__test__/browser/mcp/tools/getOpenEditorFileText.test.ts (0 hunks)
  • packages/ai-native/__test__/browser/mcp/tools/getSelectedText.test.ts (0 hunks)
  • packages/ai-native/__test__/browser/mcp/tools/replaceOpenEditorFile.test.ts (0 hunks)
  • packages/ai-native/__test__/browser/mcp/tools/replaceOpenEditorFileByDiffPreviewer.test.ts (0 hunks)
  • packages/ai-native/__test__/node/mcp-server.test.ts (1 hunks)
💤 Files with no reviewable changes (4)
  • packages/ai-native/test/browser/mcp/tools/getSelectedText.test.ts
  • packages/ai-native/test/browser/mcp/tools/getOpenEditorFileText.test.ts
  • packages/ai-native/test/browser/mcp/tools/replaceOpenEditorFileByDiffPreviewer.test.ts
  • packages/ai-native/test/browser/mcp/tools/replaceOpenEditorFile.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/ai-native/test/node/mcp-server.test.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 (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: build-windows
  • GitHub Check: unittest (macos-latest, 18.x, node)

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
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: 5

🔭 Outside diff range comments (1)
packages/ai-native/__test__/node/mcp-server.test.ts (1)

83-88: 🛠️ Refactor suggestion

测试用例需要完善

测试期望中缺少对 toolCallId 参数的验证。建议更新测试断言以确保该参数被正确传递。

 expect(mockClient.callTool).toHaveBeenCalledWith({
   name: toolName,
   arguments: { key: 'value' },
+  toolCallId: 'toolCallId',
 });
🧹 Nitpick comments (10)
packages/ai-native/src/browser/mcp/tools/grepSearch.ts (1)

52-52: 请完善 TODO 注释
此处提及需要“语义化搜索”的更多说明,但尚未完成。建议尽早补充具体描述,以便后续维护人员了解此功能的优劣势与适用场景。

packages/ai-native/src/browser/mcp/mcp-server.feature.registry.ts (1)

65-67: 建议增加类型安全性

当前从 args 中提取 toolCallId 的方式可能不够类型安全。建议定义一个接口来约束 args 的类型。

-  const toolCallId = args.toolCallId;
+  interface ToolArgs {
+    toolCallId: string;
+    [key: string]: any;
+  }
+  const { toolCallId, ...restArgs } = args as ToolArgs;
packages/ai-native/src/browser/model/msg-history-manager.ts (2)

11-11: 建议添加类型定义和文档注释

为了提高代码的可维护性和可读性,建议:

  1. messageAdditionalMap 定义具体的类型接口
  2. 为事件发射器添加 JSDoc 文档说明其用途
+interface MessageAdditional {
+  [key: string]: any;
+}
-private messageAdditionalMap: Map<string, Record<string, any>> = new Map();
+private messageAdditionalMap: Map<string, MessageAdditional> = new Map();

+/** 当消息附加数据发生变化时触发 */
private readonly _onMessageAdditionalChange = new Emitter<MessageAdditional>();

Also applies to: 16-17


79-91: 建议优化错误处理和返回值类型

建议增加以下改进:

  1. 为不存在的消息 ID 添加错误日志
  2. 明确 getMessageAdditional 的返回值类型
+import { ILogger } from '@opensumi/ide-core-common';

+@Autowired(ILogger)
+private readonly logger: ILogger;

public setMessageAdditional(id: string, additional: MessageAdditional) {
  if (!this.messageMap.has(id)) {
+   this.logger.warn(`Attempted to set additional data for non-existent message: ${id}`);
    return;
  }
  // ... rest of the code
}

-public getMessageAdditional(id: string): Record<string, any> {
+public getMessageAdditional(id: string): MessageAdditional {
  return this.messageAdditionalMap.get(id) || {};
}
packages/ai-native/src/browser/mcp/tools/components/SearchResult.tsx (1)

52-64: 优化性能

parsedFiles 的依赖项不完整,可能导致不必要的重计算。

const parsedFiles = useMemo(
  () =>
    files.map((file) => {
      const uri = URI.parse(file);
      const iconClass = labelService.getIcon(uri);
      return {
        iconClass,
        name: uri.path.base,
        path: path.relative(workspaceRoot.codeUri.fsPath, uri.path.dir.toString()),
      };
    }),
-  [files],
+  [files, labelService, workspaceRoot],
);
packages/ai-native/src/browser/mcp/tools/fileSearch.ts (2)

21-21: 建议将魔法数字提取为配置项

MAX_RESULTS = 10 建议移至配置文件中,使其更容易维护和调整。

-const MAX_RESULTS = 10;
+import { FILE_SEARCH_CONFIG } from '../config';
+const { MAX_RESULTS } = FILE_SEARCH_CONFIG;

Also applies to: 72-75


64-66: 建议将排除模式配置化

目前硬编码的排除模式 ['**/node_modules/**'] 建议移至配置文件,并考虑支持用户自定义配置。

-      excludePatterns: ['**/node_modules/**'],
+      excludePatterns: config.getExcludePatterns(),
packages/ai-native/src/common/tool-invocation-registry.ts (1)

100-107: 简化 registerTool 方法的条件逻辑

当前实现中,无论条件如何,都执行相同的 set 操作。建议简化代码以提高可读性。

建议应用以下更改:

  registerTool(tool: ToolRequest): void {
-   if (this.tools.has(tool.id)) {
-     // TODO: 使用适当的日志机制
-     this.tools.set(tool.id, tool);
-   } else {
-     this.tools.set(tool.id, tool);
-   }
+   // TODO: 如果需要,在工具已存在时添加日志
+   this.tools.set(tool.id, tool);
  }
packages/core-common/src/types/ai-native/index.ts (1)

433-434: 建议优化 additional 属性的类型定义

当前 additional 属性使用 Record<string, any> 类型过于宽松,可能导致类型安全问题。建议:

  1. 定义具体的接口来描述工具调用结果的结构
  2. 使用联合类型来限制可能的值类型

示例改进:

-  additional?: Record<string, any>;
+  additional?: {
+    toolCallResults?: {
+      toolCallId: string;
+      name: string;
+      result: string | number | boolean | null;
+    }[];
+  };
packages/ai-native/src/browser/mcp/tools/components/index.module.less (1)

100-118: .fileItem 样式与交互反馈建议改进
.fileItem 的样式定义清晰,设置了适当的内边距、字体及布局,并通过 overflow 属性处理了文本溢出。不过,在悬停状态(hover)下,背景色保持与父级背景相同,可能会导致交互反馈不够明显。
建议修改 &:hover 部分的背景色,例如使用 less 的 lighten 函数稍微调整颜色以增强视觉反馈:

-  &:hover {
-    background-color: var(--design-block-background);
-  }
+  &:hover {
+    background-color: lighten(var(--design-block-background), 10%);
+  }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 21867c6 and 008c396.

📒 Files selected for processing (27)
  • packages/ai-native/__test__/node/mcp-server.test.ts (2 hunks)
  • packages/ai-native/src/browser/components/ChatToolRender.tsx (2 hunks)
  • packages/ai-native/src/browser/index.ts (2 hunks)
  • packages/ai-native/src/browser/mcp/mcp-server.feature.registry.ts (1 hunks)
  • packages/ai-native/src/browser/mcp/tools/components/SearchResult.tsx (1 hunks)
  • packages/ai-native/src/browser/mcp/tools/components/index.module.less (1 hunks)
  • packages/ai-native/src/browser/mcp/tools/fileSearch.ts (1 hunks)
  • packages/ai-native/src/browser/mcp/tools/findFilesByNameSubstring.ts (0 hunks)
  • packages/ai-native/src/browser/mcp/tools/getCurrentFilePath.ts (0 hunks)
  • packages/ai-native/src/browser/mcp/tools/getFileTextByPath.ts (0 hunks)
  • packages/ai-native/src/browser/mcp/tools/getOpenEditorFileText.ts (0 hunks)
  • packages/ai-native/src/browser/mcp/tools/getSelectedText.ts (0 hunks)
  • packages/ai-native/src/browser/mcp/tools/grepSearch.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/mcp/tools/replaceOpenEditorFile.ts (0 hunks)
  • packages/ai-native/src/browser/mcp/tools/replaceOpenEditorFileByDiffPreviewer.ts (0 hunks)
  • packages/ai-native/src/browser/model/msg-history-manager.ts (2 hunks)
  • packages/ai-native/src/common/mcp-server-manager.ts (1 hunks)
  • packages/ai-native/src/common/tool-invocation-registry.ts (2 hunks)
  • packages/ai-native/src/node/base-language-model.ts (2 hunks)
  • packages/ai-native/src/node/mcp-server-manager-impl.ts (3 hunks)
  • packages/ai-native/src/node/mcp-server.ts (3 hunks)
  • packages/ai-native/src/node/mcp/sumi-mcp-server.ts (2 hunks)
  • packages/core-common/src/types/ai-native/index.ts (1 hunks)
  • packages/search/src/browser/search.service.ts (1 hunks)
  • packages/search/src/common/content-search.ts (1 hunks)
💤 Files with no reviewable changes (7)
  • packages/ai-native/src/browser/mcp/tools/getOpenEditorFileText.ts
  • packages/ai-native/src/browser/mcp/tools/replaceOpenEditorFile.ts
  • packages/ai-native/src/browser/mcp/tools/getSelectedText.ts
  • packages/ai-native/src/browser/mcp/tools/getCurrentFilePath.ts
  • packages/ai-native/src/browser/mcp/tools/findFilesByNameSubstring.ts
  • packages/ai-native/src/browser/mcp/tools/getFileTextByPath.ts
  • packages/ai-native/src/browser/mcp/tools/replaceOpenEditorFileByDiffPreviewer.ts
🧰 Additional context used
🪛 GitHub Check: unittest (ubuntu-latest, 18.x, node)
packages/ai-native/src/browser/mcp/tools/components/SearchResult.tsx

[failure] 3-3:
Cannot find module '@opensumi/ide-ai-native/lib/common' or its corresponding type declarations.

packages/ai-native/src/browser/mcp/tools/grepSearch.ts

[failure] 4-4:
Cannot find module '@opensumi/ide-ai-native/lib/common' or its corresponding type declarations.

packages/ai-native/src/browser/mcp/tools/fileSearch.ts

[failure] 4-4:
Cannot find module '@opensumi/ide-ai-native/lib/common' or its corresponding type declarations.

🪛 Biome (1.9.4)
packages/ai-native/src/browser/mcp/tools/grepSearch.ts

[error] 103-103: Avoid the use of spread (...) syntax on accumulators.

Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.

(lint/performance/noAccumulatingSpread)

⏰ Context from checks skipped due to timeout of 90000ms (7)
  • GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
  • GitHub Check: build (ubuntu-latest, 20.x)
  • GitHub Check: build (macos-latest, 20.x)
  • GitHub Check: unittest (macos-latest, 18.x, jsdom)
  • GitHub Check: unittest (macos-latest, 18.x, node)
  • GitHub Check: build-windows
  • GitHub Check: ubuntu-latest, Node.js 20.x
🔇 Additional comments (25)
packages/ai-native/src/browser/mcp/tools/grepSearch.ts (2)

4-4: 无法找到对应模块或类型声明
可能是路径错误或缺少类型声明文件,导致无法解析 @opensumi/ide-ai-native/lib/common。建议检查依赖包和 tsconfig 中的路径配置,确保导入路径或类型声明可用。

🧰 Tools
🪛 GitHub Check: unittest (ubuntu-latest, 18.x, node)

[failure] 4-4:
Cannot find module '@opensumi/ide-ai-native/lib/common' or its corresponding type declarations.


42-45: 工具注册逻辑看起来可行
此处将搜索工具注册到 MCP 平台,并指定了前端呈现组件,整体实现较为规范,暂未发现明显问题。

packages/ai-native/src/common/mcp-server-manager.ts (1)

4-9: 新增 toolCallId 参数需要确认全局引用
callTool 方法签名新增了 toolCallId。为确保功能正常,请确认所有调用点都已传入相应的参数,并对其进行测试以避免潜在的运行时错误。

packages/ai-native/src/browser/mcp/tools/listDir.ts (1)

17-17: explanation 字段改为可选
此变更提升了灵活性,允许在不需要说明的情况下也可使用此工具,避免不必要的字段校验错误。整体实现无异常,值得肯定。

packages/ai-native/src/browser/mcp/tools/readFile.ts (1)

16-19: 代码变更合理

explanation 字段设置为可选是一个合理的改进,这样可以在不需要解释的情况下简化工具的使用。

packages/ai-native/src/node/mcp-server.ts (2)

11-11: 接口更新符合预期!

接口方法签名的更新正确地包含了新的 toolCallId 参数,这有助于工具调用的追踪和管理。


90-107: 实现更新完整且一致!

callTool 方法的实现正确地处理了新增的 toolCallId 参数,并将其包含在传递给客户端的参数对象中。

packages/ai-native/src/browser/components/ChatToolRender.tsx (1)

57-95: 组件渲染逻辑优化!

使用三元运算符重构了渲染逻辑,使代码更加简洁清晰。同时正确地传递了 toolCallId 到 ToolComponent。

packages/ai-native/src/common/tool-invocation-registry.ts (2)

6-13: 代码结构清晰且类型安全!

使用 zod 进行参数验证的实现非常合理,特别是通过 lazy 实现的递归定义可以很好地处理嵌套结构。


17-24: 接口设计合理且保持了向后兼容性!

handler 方法签名的更新很好地支持了工具执行选项,同时通过可选参数保持了与现有代码的兼容性。

packages/ai-native/src/node/mcp-server-manager-impl.ts (2)

47-58: 方法签名更新合理且实现正确!

callTool 方法的更新很好地支持了工具调用 ID 的传递,实现逻辑清晰。


81-91: 错误处理完善,参数传递正确!

handler 实现很好地处理了新增的 options 参数,同时保持了原有的错误处理逻辑。

packages/ai-native/src/node/base-language-model.ts (1)

69-70: 参数传递实现正确且符合功能需求!

execute 函数的更新正确地将 options 参数传递给了 handler,实现符合预期。

packages/ai-native/src/node/mcp/sumi-mcp-server.ts (1)

159-178: 参数处理和错误处理都很完善!

callTool 方法的实现正确地处理了 toolCallId 参数,并保持了良好的 JSON 解析错误处理。

packages/ai-native/src/browser/index.ts (2)

61-61: 导入语句的顺序符合规范!

新增的 FileSearchToolGrepSearchTool 的导入语句按字母顺序正确放置。

Also applies to: 64-64


100-101: 工具注册顺序合理!

新增的搜索工具 FileSearchToolGrepSearchTool 被正确地添加到 MCP Server Contributions 区域,并保持了与其他工具一致的注册方式。

packages/search/src/common/content-search.ts (1)

151-151: 文档注释更新准确!

lineText 属性的注释从 rangeLineText 更新为 renderLineText,与实际代码实现保持一致。这个更新提高了文档的准确性。

packages/search/src/browser/search.service.ts (2)

235-239: 搜索参数扩展设计合理!

doSearch 方法通过扩展 state 参数类型来支持更灵活的搜索配置,同时保持了向后兼容性。这种设计方式值得肯定。


241-241: 参数处理逻辑完善!

新增参数的处理逻辑考虑周全:

  1. maxResults 有合理的默认值 2000
  2. includeexclude 在未提供时会回退到实例变量

Also applies to: 247-248

packages/ai-native/src/browser/mcp/tools/components/index.module.less (6)

69-74: .container 样式定义清晰
.container 类为容器提供了明确的边框、圆角和溢出隐藏效果,有助于组件的视觉分组和框架整洁。目前的实现符合设计预期。


76-86: .header 样式设置得当
.header 类通过合理的内边距、背景色、边框底部和对齐方式,构建了一个易于点击且与整体风格一致的头部区域。


88-94: .fileList 样式完善
.fileList 定义了无内外边距、无列表样式的块级展示,并设置了背景色,保证了文件列表展示的整洁性。


96-98: .fileList.collapsed 状态实现
通过将 display 设置为 none,实现了列表的折叠效果,方案简单明了,符合需求。


120-124: .fileIcon 样式检查
.fileIcon 类适当地设置了图标的颜色和尺寸,有助于与文本内容形成区分,提升整体视觉效果。


126-132: .filePath 样式实现良好
.filePath 类利用 flex 布局及自动边距,实现了文本的对齐和自适应分配空间,样式清晰且实用。

Copy link
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: 2

♻️ Duplicate comments (1)
packages/ai-native/src/browser/mcp/tools/grepSearch.ts (1)

97-103: 🛠️ Refactor suggestion

优化搜索结果处理性能

.reduce() 中使用扩展运算符会导致 O(n²) 的时间复杂度。建议使用 push 方法来提高性能。

-.reduce((acc, r) => {
-  if (acc.find((a) => a.line === r.line)) {
-    return acc;
-  }
-  return [...acc, r];
-}, [] as ContentSearchResult[])
+.reduce((acc, r) => {
+  if (!acc.some((a) => a.line === r.line)) {
+    acc.push(r);
+  }
+  return acc;
+}, [] as ContentSearchResult[])
🧰 Tools
🪛 Biome (1.9.4)

[error] 103-103: Avoid the use of spread (...) syntax on accumulators.

Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.

(lint/performance/noAccumulatingSpread)

🧹 Nitpick comments (4)
packages/ai-native/src/browser/mcp/tools/components/SearchResult.tsx (2)

12-17: 建议改进类型安全性

args 参数使用 any 类型会降低类型安全性,建议定义具体的接口类型。

interface SearchResultProps {
-  args: any;
+  args: {
+    query: string;
+    explanation?: string;
+  };
  toolCallId: string;
  messageId: string;
  toolName: string;
}

19-25: 建议重构以减少代码重复

FileSearchToolComponentGrepSearchToolComponent 结构完全相同,可以合并为一个组件。

-export const FileSearchToolComponent: React.FC<SearchResultProps> = ({ args, toolCallId, messageId }) => (
-  <SearchResult args={args} toolCallId={toolCallId} messageId={messageId} toolName='fileSearch' />
-);
-
-export const GrepSearchToolComponent: React.FC<SearchResultProps> = ({ args, toolCallId, messageId }) => (
-  <SearchResult args={args} toolCallId={toolCallId} messageId={messageId} toolName='grepSearch' />
-);
+export const SearchToolComponent = (toolName: 'fileSearch' | 'grepSearch') =>
+  React.memo<Omit<SearchResultProps, 'toolName'>>(({ args, toolCallId, messageId }) => (
+    <SearchResult args={args} toolCallId={toolCallId} messageId={messageId} toolName={toolName} />
+  ));
+
+export const FileSearchToolComponent = SearchToolComponent('fileSearch');
+export const GrepSearchToolComponent = SearchToolComponent('grepSearch');
packages/ai-native/src/browser/mcp/tools/fileSearch.ts (1)

64-66: 建议改进配置管理

当前的忽略配置是硬编码的,建议将其移至配置文件中以提高灵活性。

-      // TODO: 忽略配置
-      excludePatterns: ['**/node_modules/**'],
+      excludePatterns: this.getExcludePatterns(),

建议添加以下方法:

private getExcludePatterns(): string[] {
  return this.configService.getValue('search.exclude') || ['**/node_modules/**'];
}
packages/ai-native/src/browser/mcp/tools/grepSearch.ts (1)

52-53: 需要更新工具描述

TODO 注释表明需要在支持语义化搜索后更新描述。建议创建一个任务来跟踪这个改进。

是否需要我帮助创建一个任务来跟踪语义化搜索的实现和文档更新?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 008c396 and 0de192a.

📒 Files selected for processing (3)
  • packages/ai-native/src/browser/mcp/tools/components/SearchResult.tsx (1 hunks)
  • packages/ai-native/src/browser/mcp/tools/fileSearch.ts (1 hunks)
  • packages/ai-native/src/browser/mcp/tools/grepSearch.ts (1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/ai-native/src/browser/mcp/tools/grepSearch.ts

[error] 103-103: Avoid the use of spread (...) syntax on accumulators.

Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.

(lint/performance/noAccumulatingSpread)

⏰ Context from checks skipped due to timeout of 90000ms (8)
  • GitHub Check: build (ubuntu-latest, 20.x)
  • GitHub Check: build (macos-latest, 20.x)
  • GitHub Check: build-windows
  • 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: ubuntu-latest, Node.js 20.x
  • GitHub Check: unittest (macos-latest, 18.x, node)
🔇 Additional comments (2)
packages/ai-native/src/browser/mcp/tools/components/SearchResult.tsx (2)

38-45: 优化 useEffect 清理逻辑

当前的 useEffect 清理可能存在内存泄漏风险。建议在组件卸载时确保所有订阅都被正确清理。


47-50: 添加错误处理

文件打开操作缺少错误处理机制。

Copy link
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: 0

🧹 Nitpick comments (6)
packages/ai-native/__test__/common/mcp-server-manager.test.ts (1)

88-89: 建议改进测试用例的实现方式

当前测试用例存在以下可以改进的地方:

  1. 'call-x' 是硬编码的值,建议将其定义为有意义的常量或变量
  2. 缺少对错误情况的测试,比如当 toolCallId 为空或无效时的处理
  3. 测试用例的名称可以更具体地反映新增的 toolCallId 参数

建议按照以下方式重构测试代码:

+  const TEST_TOOL_CALL_ID = 'test-call-123';
+
-  it('should call tool on server', async () => {
+  it('should call tool on server with valid tool call ID', async () => {
     const toolName = 'test-tool';
     const argString = '{"key": "value"}';
-    await mockManager.callTool('test-server', toolName, 'call-x', argString);
-    expect(mockManager.callTool).toHaveBeenCalledWith('test-server', toolName, 'call-x', argString);
+    await mockManager.callTool('test-server', toolName, TEST_TOOL_CALL_ID, argString);
+    expect(mockManager.callTool).toHaveBeenCalledWith('test-server', toolName, TEST_TOOL_CALL_ID, argString);
   });
+
+  it('should reject invalid tool call ID', async () => {
+    const toolName = 'test-tool';
+    const argString = '{"key": "value"}';
+    await expect(
+      mockManager.callTool('test-server', toolName, '', argString)
+    ).rejects.toThrow('Invalid tool call ID');
+  });
packages/ai-native/src/browser/mcp/tools/components/SearchResult.tsx (2)

12-17: 改进类型定义和组件结构

建议进行以下改进:

  1. args 的类型从 any 改为具体的接口类型
  2. 考虑合并两个工具组件,通过属性区分不同工具类型
interface SearchResultProps {
-  args: any;
+  args: {
+    query: string;
+    [key: string]: unknown;
+  };
  toolCallId: string;
  messageId: string;
  toolName: string;
}

81-81: 改进文件路径处理逻辑

建议使用更安全的路径处理方式:

  1. 验证工作区根路径是否存在
  2. 使用 path.join 替代字符串拼接
- onClick={() => handleFileClick(URI.file(path.join(workspaceRoot.codeUri.fsPath, file.path, file.name)))}
+ onClick={() => {
+   if (!workspaceRoot?.codeUri?.fsPath) {
+     return;
+   }
+   const filePath = path.join(workspaceRoot.codeUri.fsPath, file.path, file.name);
+   handleFileClick(URI.file(filePath));
+ }}
packages/ai-native/src/browser/mcp/tools/fileSearch.ts (2)

64-70: 优化搜索配置的灵活性

当前的搜索配置存在以下问题:

  1. 硬编码的排除模式 **/node_modules/**
  2. 未完成的忽略配置 TODO

建议:

  1. 从工作区配置或用户设置中读取排除模式
  2. 完成忽略配置的实现

需要我帮助实现配置读取和忽略规则的逻辑吗?


62-70: 添加错误处理和进度反馈

建议添加以下功能:

  1. 搜索操作的错误处理
  2. 搜索进度的反馈机制
const searchResults = await this.fileSearchService.find(searchPattern, {
  rootUris: [new URI(workspaceRoots[0].uri).codeUri.fsPath],
  excludePatterns: ['**/node_modules/**'],
  limit: 100,
  useGitIgnore: true,
  noIgnoreParent: true,
  fuzzyMatch: true,
+ onProgress: (progress) => {
+   logger.appendLine(`搜索进度: ${progress}%`);
+ },
}).catch(error => {
+  logger.appendLine(`搜索失败: ${error.message}`);
+  throw error;
});
packages/ai-native/src/browser/mcp/tools/grepSearch.ts (1)

52-53: 完善语义化搜索的描述

TODO 注释提到需要描述语义化搜索的优劣势。

需要我帮助编写关于语义化搜索功能的详细描述吗?我可以提供:

  1. 与正则搜索的对比
  2. 使用场景建议
  3. 性能考虑因素
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0de192a and 4e9c60d.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (5)
  • packages/ai-native/__test__/common/mcp-server-manager.test.ts (1 hunks)
  • packages/ai-native/package.json (1 hunks)
  • packages/ai-native/src/browser/mcp/tools/components/SearchResult.tsx (1 hunks)
  • packages/ai-native/src/browser/mcp/tools/fileSearch.ts (1 hunks)
  • packages/ai-native/src/browser/mcp/tools/grepSearch.ts (1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/ai-native/src/browser/mcp/tools/grepSearch.ts

[error] 103-103: Avoid the use of spread (...) syntax on accumulators.

Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.

(lint/performance/noAccumulatingSpread)

⏰ 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: build (ubuntu-latest, 20.x)
  • GitHub Check: unittest (macos-latest, 18.x, node)
  • GitHub Check: build (macos-latest, 20.x)
  • GitHub Check: build-windows
  • GitHub Check: ubuntu-latest, Node.js 20.x
🔇 Additional comments (5)
packages/ai-native/src/browser/mcp/tools/components/SearchResult.tsx (2)

38-45: 优化 useEffect 依赖项和清理逻辑


47-50: 添加文件操作的错误处理

packages/ai-native/src/browser/mcp/tools/grepSearch.ts (2)

97-103: 避免在 .reduce() 中使用扩展运算符

🧰 Tools
🪛 Biome (1.9.4)

[error] 103-103: Avoid the use of spread (...) syntax on accumulators.

Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.

(lint/performance/noAccumulatingSpread)


87-91: 添加搜索超时处理

packages/ai-native/package.json (1)

41-41: 依赖添加正确

新增的 @opensumi/ide-search 依赖与新的搜索功能相符。

Aaaaash
Aaaaash previously approved these changes Feb 21, 2025
Copy link
Member

@Aaaaash Aaaaash left a comment

Choose a reason for hiding this comment

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

LGTM

Ricbet
Ricbet previously approved these changes Feb 21, 2025
Copy link

codecov bot commented Feb 21, 2025

Codecov Report

Attention: Patch coverage is 3.24675% with 149 lines in your changes missing coverage. Please review.

Project coverage is 53.58%. Comparing base (21867c6) to head (325fb67).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...ages/ai-native/src/browser/mcp/tools/grepSearch.ts 0.00% 45 Missing and 5 partials ⚠️
...s/ai-native/src/common/tool-invocation-registry.ts 0.00% 35 Missing and 4 partials ⚠️
...ages/ai-native/src/browser/mcp/tools/fileSearch.ts 0.00% 35 Missing and 2 partials ⚠️
...ai-native/src/browser/model/msg-history-manager.ts 0.00% 9 Missing and 2 partials ⚠️
...ages/ai-native/src/node/mcp-server-manager-impl.ts 0.00% 3 Missing and 1 partial ⚠️
packages/ai-native/src/browser/index.ts 0.00% 2 Missing ⚠️
...ive/src/browser/mcp/mcp-server.feature.registry.ts 0.00% 2 Missing ⚠️
packages/ai-native/src/node/base-language-model.ts 0.00% 2 Missing ⚠️
packages/ai-native/src/node/mcp/sumi-mcp-server.ts 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4396      +/-   ##
==========================================
- Coverage   53.73%   53.58%   -0.16%     
==========================================
  Files        1655     1651       -4     
  Lines      101889   101806      -83     
  Branches    22028    22027       -1     
==========================================
- Hits        54751    54552     -199     
- Misses      39203    39308     +105     
- Partials     7935     7946      +11     
Flag Coverage Δ
jsdom 49.08% <2.59%> (-0.16%) ⬇️
node 12.23% <0.64%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ensorrow ensorrow merged commit 7094c8c into main Feb 21, 2025
12 checks passed
@ensorrow ensorrow deleted the feat/search-tool branch February 21, 2025 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎨 feature feature required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants