feat(agent): support inquiry agent mode when notepad created - #10
Closed
emikeliu wants to merge 4 commits into
Closed
feat(agent): support inquiry agent mode when notepad created#10emikeliu wants to merge 4 commits into
emikeliu wants to merge 4 commits into
Conversation
审阅者指南在整个扩展中增加了本地化的 i18n 层，引入了新的 用于展示当前代理模式的 mutsumi.displayMode 命令时序图sequenceDiagram
actor User
participant VSCode as VSCode
participant Extension as MutsumiExtension
participant Notebook as mutsumi-notebook
User->>VSCode: executeCommand(mutsumi.displayMode)
VSCode->>Extension: registerModeDisplayCommand handler
alt [no active notebook editor]
Extension->>vscode.window: showWarningMessage(t(notebook.noEditor))
else [notebookType != mutsumi-notebook]
Extension->>vscode.window: showWarningMessage(t(notebook.onlyMutsumi))
else [valid mutsumi-notebook]
Extension->>Notebook: read notebook.metadata.contextItems
Extension->>vscode.window: showInformationMessage(t(modeDisplay.currentMode, mode))
end
文件级变更
提示和命令与 Sourcery 交互
自定义你的使用体验打开你的控制面板 来：
获取帮助Original review guide in EnglishReviewer's GuideAdds a localized i18n layer across the extension, introduces a new Sequence diagram for mutsumi.displayMode command to show current agent modesequenceDiagram
actor User
participant VSCode as VSCode
participant Extension as MutsumiExtension
participant Notebook as mutsumi-notebook
User->>VSCode: executeCommand(mutsumi.displayMode)
VSCode->>Extension: registerModeDisplayCommand handler
alt [no active notebook editor]
Extension->>vscode.window: showWarningMessage(t(notebook.noEditor))
else [notebookType != mutsumi-notebook]
Extension->>vscode.window: showWarningMessage(t(notebook.onlyMutsumi))
else [valid mutsumi-notebook]
Extension->>Notebook: read notebook.metadata.contextItems
Extension->>vscode.window: showInformationMessage(t(modeDisplay.currentMode, mode))
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了 4 个问题，并留下了一些总体反馈：
- 新的
mutsumi.displayMode命令目前假设当前模式存储在metadata.contextItems[0].content中；建议使用一个更明确、定义良好的 metadata 字段，这样在contextItems结构变更时，这个逻辑仍然能保持稳定。 - 在
modeDisplay.ts中，showInformationMessage使用了{ modal: true }，会阻塞 UI；如果这只是用于展示信息，建议改为非模态通知，以避免打断用户的工作流。 - 用于调试输出通道名称的 i18n key（
t('Mutsumi Debug')）是一个字面量，而不是像其他字符串那样使用命名空间的 key；建议将其与现有 key 命名约定对齐（例如debugLogger.channelName），这样更便于管理本地化资源包。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `mutsumi.displayMode` command currently assumes the current mode is stored as `metadata.contextItems[0].content`; consider using a more explicit, well-defined metadata field so this remains stable if contextItems structure changes.
- In `modeDisplay.ts`, `showInformationMessage` is called with `{ modal: true }` which blocks the UI; if this is only informational, you may want to use a non-modal notification to avoid interrupting the workflow.
- The i18n key used for the debug output channel name (`t('Mutsumi Debug')`) is a literal rather than a namespaced key like other strings; aligning it with the key naming convention (e.g. `debugLogger.channelName`) will make localization bundles easier to manage.
## Individual Comments
### Comment 1
<location path="src/extension.ts" line_range="64-73" />
<code_context>
+ * between extension activation and the onDidChangeConfiguration handler so
+ * that behavior stays consistent.
+ */
+function initializeAgentTypeSystem(): void {
+ // 1. Load Mutsumi configuration (merged with built-in defaults + validated)
+ const mutsumiConfig = loadMutsumiConfig();
+ debugLogger.log("[Extension] Mutsumi config loaded successfully");
+
+ // 2. Initialize ToolSetRegistry with configured tool sets
+ const toolSetRegistry = ToolSetRegistry.getInstance();
+ toolSetRegistry.initialize(mutsumiConfig.toolSets);
+ debugLogger.log("[Extension] ToolSetRegistry initialized");
+
+ // 3. Initialize AgentTypeRegistry with configured agent types
+ const agentTypeRegistry = AgentTypeRegistry.getInstance();
+ agentTypeRegistry.initialize(
+ mutsumiConfig.agentTypes,
</code_context>
<issue_to_address>
**issue (bug_risk):** AgentTypeSystem reload can leave registries in an unusable state if initialization throws.
Because both registries now clear internal state before validating the new config, any exception in `loadMutsumiConfig()` or either `initialize()` (e.g. malformed `mutsumi.agentConfig`) leaves them with `ready = false` and empty data, breaking subsequent agent operations after a bad config change. Consider validating against a cloned copy of the state, then only swapping the internal maps/flags once validation fully succeeds; on failure, retain the previous configuration and surface the error instead of partially resetting the registries.
</issue_to_address>
### Comment 2
<location path="src/registry/toolSetRegistry.ts" line_range="69-71" />
<code_context>
* @throws {Error} If a tool set references a non-existent tool
*/
initialize(config: ToolSetsConfig): void {
- if (this.initialized) {
- return;
- }
-
- // Clear existing tool sets
+ // Clear existing tool sets so a reload fully replaces prior state
this.toolSets.clear();
// Check if RAG is enabled (embedding endpoint configured)
</code_context>
<issue_to_address>
**issue (bug_risk):** ToolSetRegistry.initialize clears state before validation, which exacerbates reload failure risk.
With the new reload behavior, `initialize()` now clears `this.toolSets` before validation and only later sets `this.ready = true`. If any validation fails (missing tools, RAG checks, or future logic), the registry is left with an empty map and `ready` may never be set, breaking callers like `getToolSet` / `getCombinedToolSet` after an invalid config change.
Consider instead constructing and validating a new `Map<string, string[]>` locally, then assigning it to `this.toolSets` and setting `this.ready = true` only after validation fully succeeds. This preserves the previous, valid tool sets when a reload fails.
</issue_to_address>
### Comment 3
<location path="src/registry/agentTypeRegistry.ts" line_range="51-53" />
<code_context>
* @throws {Error} If validation fails
*/
initialize(config: AgentTypeConfigMap, toolSetNames: string[]): void {
- if (this.initialized) {
- return;
- }
-
- // Clear existing types
+ // Clear existing types so a reload fully replaces prior state
this.agentTypes.clear();
this.toolSetNames = new Set(toolSetNames);
</code_context>
<issue_to_address>
**issue (bug_risk):** AgentTypeRegistry.initialize now fully clears types before validation, similar reload robustness concern.
Because `initialize()` clears `agentTypes` and resets `toolSetNames` before validation, any validation failure (e.g. unknown child types or tool set references) leaves the registry empty and `ready = false`, breaking consumers like `getEntryAgentTypes` and `getAgentType`. Consider validating into temporary `Map`/`Set` instances and only swapping them in and setting `ready = true` after validation succeeds, so the last-known-good configuration is preserved when `mutsumi.agentConfig` is invalid.
</issue_to_address>
### Comment 4
<location path="src/notebook/commands/modeDisplay.ts" line_range="1-3" />
<code_context>
import { normalizeReasoningEffort } from './agent/types';
+import { t } from './i18n';
/**
* Controls the execution of agent notebooks.
</code_context>
<issue_to_address>
**nitpick:** Mode display command file header still references selectModel, which can confuse maintenance.
Please update the file-level JSDoc (description and `@module` tag) to reflect `mutsumi.displayMode` / `modeDisplay` so they align with the current command implementation and avoid confusion for future readers and tooling that uses these annotations.
</issue_to_address>Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Original comment in English
Hey - I've found 4 issues, and left some high level feedback:
- The new
mutsumi.displayModecommand currently assumes the current mode is stored asmetadata.contextItems[0].content; consider using a more explicit, well-defined metadata field so this remains stable if contextItems structure changes. - In
modeDisplay.ts,showInformationMessageis called with{ modal: true }which blocks the UI; if this is only informational, you may want to use a non-modal notification to avoid interrupting the workflow. - The i18n key used for the debug output channel name (
t('Mutsumi Debug')) is a literal rather than a namespaced key like other strings; aligning it with the key naming convention (e.g.debugLogger.channelName) will make localization bundles easier to manage.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `mutsumi.displayMode` command currently assumes the current mode is stored as `metadata.contextItems[0].content`; consider using a more explicit, well-defined metadata field so this remains stable if contextItems structure changes.
- In `modeDisplay.ts`, `showInformationMessage` is called with `{ modal: true }` which blocks the UI; if this is only informational, you may want to use a non-modal notification to avoid interrupting the workflow.
- The i18n key used for the debug output channel name (`t('Mutsumi Debug')`) is a literal rather than a namespaced key like other strings; aligning it with the key naming convention (e.g. `debugLogger.channelName`) will make localization bundles easier to manage.
## Individual Comments
### Comment 1
<location path="src/extension.ts" line_range="64-73" />
<code_context>
+ * between extension activation and the onDidChangeConfiguration handler so
+ * that behavior stays consistent.
+ */
+function initializeAgentTypeSystem(): void {
+ // 1. Load Mutsumi configuration (merged with built-in defaults + validated)
+ const mutsumiConfig = loadMutsumiConfig();
+ debugLogger.log("[Extension] Mutsumi config loaded successfully");
+
+ // 2. Initialize ToolSetRegistry with configured tool sets
+ const toolSetRegistry = ToolSetRegistry.getInstance();
+ toolSetRegistry.initialize(mutsumiConfig.toolSets);
+ debugLogger.log("[Extension] ToolSetRegistry initialized");
+
+ // 3. Initialize AgentTypeRegistry with configured agent types
+ const agentTypeRegistry = AgentTypeRegistry.getInstance();
+ agentTypeRegistry.initialize(
+ mutsumiConfig.agentTypes,
</code_context>
<issue_to_address>
**issue (bug_risk):** AgentTypeSystem reload can leave registries in an unusable state if initialization throws.
Because both registries now clear internal state before validating the new config, any exception in `loadMutsumiConfig()` or either `initialize()` (e.g. malformed `mutsumi.agentConfig`) leaves them with `ready = false` and empty data, breaking subsequent agent operations after a bad config change. Consider validating against a cloned copy of the state, then only swapping the internal maps/flags once validation fully succeeds; on failure, retain the previous configuration and surface the error instead of partially resetting the registries.
</issue_to_address>
### Comment 2
<location path="src/registry/toolSetRegistry.ts" line_range="69-71" />
<code_context>
* @throws {Error} If a tool set references a non-existent tool
*/
initialize(config: ToolSetsConfig): void {
- if (this.initialized) {
- return;
- }
-
- // Clear existing tool sets
+ // Clear existing tool sets so a reload fully replaces prior state
this.toolSets.clear();
// Check if RAG is enabled (embedding endpoint configured)
</code_context>
<issue_to_address>
**issue (bug_risk):** ToolSetRegistry.initialize clears state before validation, which exacerbates reload failure risk.
With the new reload behavior, `initialize()` now clears `this.toolSets` before validation and only later sets `this.ready = true`. If any validation fails (missing tools, RAG checks, or future logic), the registry is left with an empty map and `ready` may never be set, breaking callers like `getToolSet` / `getCombinedToolSet` after an invalid config change.
Consider instead constructing and validating a new `Map<string, string[]>` locally, then assigning it to `this.toolSets` and setting `this.ready = true` only after validation fully succeeds. This preserves the previous, valid tool sets when a reload fails.
</issue_to_address>
### Comment 3
<location path="src/registry/agentTypeRegistry.ts" line_range="51-53" />
<code_context>
* @throws {Error} If validation fails
*/
initialize(config: AgentTypeConfigMap, toolSetNames: string[]): void {
- if (this.initialized) {
- return;
- }
-
- // Clear existing types
+ // Clear existing types so a reload fully replaces prior state
this.agentTypes.clear();
this.toolSetNames = new Set(toolSetNames);
</code_context>
<issue_to_address>
**issue (bug_risk):** AgentTypeRegistry.initialize now fully clears types before validation, similar reload robustness concern.
Because `initialize()` clears `agentTypes` and resets `toolSetNames` before validation, any validation failure (e.g. unknown child types or tool set references) leaves the registry empty and `ready = false`, breaking consumers like `getEntryAgentTypes` and `getAgentType`. Consider validating into temporary `Map`/`Set` instances and only swapping them in and setting `ready = true` after validation succeeds, so the last-known-good configuration is preserved when `mutsumi.agentConfig` is invalid.
</issue_to_address>
### Comment 4
<location path="src/notebook/commands/modeDisplay.ts" line_range="1-3" />
<code_context>
import { normalizeReasoningEffort } from './agent/types';
+import { t } from './i18n';
/**
* Controls the execution of agent notebooks.
</code_context>
<issue_to_address>
**nitpick:** Mode display command file header still references selectModel, which can confuse maintenance.
Please update the file-level JSDoc (description and `@module` tag) to reflect `mutsumi.displayMode` / `modeDisplay` so they align with the current command implementation and avoid confusion for future readers and tooling that uses these annotations.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+64
to
+73
| function initializeAgentTypeSystem(): void { | ||
| // 1. Load Mutsumi configuration (merged with built-in defaults + validated) | ||
| const mutsumiConfig = loadMutsumiConfig(); | ||
| debugLogger.log("[Extension] Mutsumi config loaded successfully"); | ||
|
|
||
| // 2. Initialize ToolSetRegistry with configured tool sets | ||
| const toolSetRegistry = ToolSetRegistry.getInstance(); | ||
| toolSetRegistry.initialize(mutsumiConfig.toolSets); | ||
| debugLogger.log("[Extension] ToolSetRegistry initialized"); | ||
|
|
There was a problem hiding this comment.
issue (bug_risk): AgentTypeSystem 的重新加载在初始化抛出异常时，可能会让多个注册表处于不可用状态。
由于这两个注册表现在会在验证新配置之前先清空内部状态，如果 loadMutsumiConfig() 或任一 initialize()（例如配置中的 mutsumi.agentConfig 格式错误）抛出异常，它们就会被置为 ready = false 且数据为空，在一次错误的配置变更之后会导致后续的 agent 操作无法正常工作。建议针对现有状态的克隆副本进行验证，并仅在验证完全成功后再替换内部的 map / 标志位；验证失败时保留之前的配置，并将错误显式暴露出来，而不是只把注册表部分重置到一个不可用的状态。
Original comment in English
issue (bug_risk): AgentTypeSystem reload can leave registries in an unusable state if initialization throws.
Because both registries now clear internal state before validating the new config, any exception in loadMutsumiConfig() or either initialize() (e.g. malformed mutsumi.agentConfig) leaves them with ready = false and empty data, breaking subsequent agent operations after a bad config change. Consider validating against a cloned copy of the state, then only swapping the internal maps/flags once validation fully succeeds; on failure, retain the previous configuration and surface the error instead of partially resetting the registries.
Comment on lines
69
to
71
| initialize(config: ToolSetsConfig): void { | ||
| if (this.initialized) { | ||
| return; | ||
| } | ||
|
|
||
| // Clear existing tool sets | ||
| // Clear existing tool sets so a reload fully replaces prior state | ||
| this.toolSets.clear(); |
There was a problem hiding this comment.
issue (bug_risk): ToolSetRegistry.initialize 在验证之前清空状态，这会加剧重新加载失败时的风险。
在新的重新加载行为下，initialize() 现在会在验证之前清空 this.toolSets，并且要到后面才设置 this.ready = true。如果验证中的任何一步失败（缺少工具、RAG 检查或未来的其他逻辑），注册表会被留在一个空的 map 中，而且 ready 可能永远不会被设置，在一次无效配置变更后会使像 getToolSet / getCombinedToolSet 这样的调用方出现问题。
建议改为先在本地构造并验证一个新的 Map<string, string[]>，只在验证完全成功后，再把它赋值给 this.toolSets 并设置 this.ready = true。这样可以在重新加载失败时保留之前有效的工具集合。
Original comment in English
issue (bug_risk): ToolSetRegistry.initialize clears state before validation, which exacerbates reload failure risk.
With the new reload behavior, initialize() now clears this.toolSets before validation and only later sets this.ready = true. If any validation fails (missing tools, RAG checks, or future logic), the registry is left with an empty map and ready may never be set, breaking callers like getToolSet / getCombinedToolSet after an invalid config change.
Consider instead constructing and validating a new Map<string, string[]> locally, then assigning it to this.toolSets and setting this.ready = true only after validation fully succeeds. This preserves the previous, valid tool sets when a reload fails.
Comment on lines
51
to
53
| initialize(config: AgentTypeConfigMap, toolSetNames: string[]): void { | ||
| if (this.initialized) { | ||
| return; | ||
| } | ||
|
|
||
| // Clear existing types | ||
| // Clear existing types so a reload fully replaces prior state | ||
| this.agentTypes.clear(); |
There was a problem hiding this comment.
issue (bug_risk): AgentTypeRegistry.initialize 在验证之前就完全清空已有类型，在重新加载健壮性方面存在类似的问题。
由于 initialize() 会在验证之前清空 agentTypes，并重置 toolSetNames，任何验证失败（例如未知的子类型或工具集引用）都会让注册表变为空且 ready = false，从而影响 getEntryAgentTypes 和 getAgentType 等使用方。建议将验证过程放在临时的 Map / Set 实例中进行，仅在验证成功之后再替换内部数据结构并设置 ready = true，这样在 mutsumi.agentConfig 无效时能够保留最近一次的有效配置。
Original comment in English
issue (bug_risk): AgentTypeRegistry.initialize now fully clears types before validation, similar reload robustness concern.
Because initialize() clears agentTypes and resets toolSetNames before validation, any validation failure (e.g. unknown child types or tool set references) leaves the registry empty and ready = false, breaking consumers like getEntryAgentTypes and getAgentType. Consider validating into temporary Map/Set instances and only swapping them in and setting ready = true after validation succeeds, so the last-known-good configuration is preserved when mutsumi.agentConfig is invalid.
Comment on lines
+1
to
+3
| /** | ||
| * @fileoverview Model selection command for Mutsumi notebook. | ||
| * @module notebook/commands/selectModel |
There was a problem hiding this comment.
nitpick: 模式显示命令文件的头部注释仍然引用了 selectModel，可能会给后续维护带来困惑。
请更新文件级的 JSDoc（描述和 @module 标签），使其反映 mutsumi.displayMode / modeDisplay，这样它们就能与当前命令实现保持一致，也能避免给未来的读者以及依赖这些注释的工具造成混淆。
Original comment in English
nitpick: Mode display command file header still references selectModel, which can confuse maintenance.
Please update the file-level JSDoc (description and @module tag) to reflect mutsumi.displayMode / modeDisplay so they align with the current command implementation and avoid confusion for future readers and tooling that uses these annotations.
Contributor
Author
|
采用更好的方式实现。 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixed issue #9.
实现了 registerModeDisplayCommand, modeDisplay 以在 notebook 已被创建时查询当前的 agent mode
请注意该 pull request 包含 #8 所提到的本地化功能
Summary by Sourcery
在整个扩展中引入本地化支持，并新增一个命令，用于在已有笔记本中显示当前代理模式，同时使代理类型系统在配置变更时可重新加载。
新功能：
增强：
mutsumi.agentConfig设置变更时自动重新加载。package.json占位符和特定语言的 nls 文件实现扩展元数据和配置描述的本地化。构建：
Original summary in English
Summary by Sourcery
Introduce localization support across the extension and add a command to display the current agent mode in existing notebooks, while making the agent type system re-loadable on configuration changes.
New Features:
Enhancements:
Build: