Merged
Conversation
Contributor
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
✅ Files skipped from review due to trivial changes (12)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds standalone image generation for DeepChat agents: presenter streaming, a new agent MCP tool, image-preview promotion into assistant messages, agent config and settings UI support (with i18n), runtime wiring, and tests. ChangesStandalone Image Generation Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/settings/components/DeepChatAgentsSettings.vue (1)
716-723:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a localized label mapping for
agent-image-generationtool group.
GROUP_ORDERincludesagent-image-generation, butgetGroupLabelhas no matching case, so the UI can fall back to raw group key text.💡 Proposed fix
const getGroupLabel = (serverName: string) => { switch (serverName) { case 'agent-filesystem': return t('chat.input.tools.groups.agentFilesystem') case 'agent-core': return t('chat.input.tools.groups.agentCore') + case 'agent-image-generation': + return t('chat.input.tools.groups.agentImageGeneration') case 'agent-skills': return t('chat.input.tools.groups.agentSkills') case 'deepchat-settings': return t('chat.input.tools.groups.deepchatSettings') case 'yobrowser':🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/settings/components/DeepChatAgentsSettings.vue` around lines 716 - 723, getGroupLabel is missing a mapping for the 'agent-image-generation' key from GROUP_ORDER, causing the raw group key to display; update the getGroupLabel function to return a localized label for 'agent-image-generation' (using the same translation function/locale pattern as other cases) so the UI shows a human-friendly name for that tool group.
🧹 Nitpick comments (1)
test/main/presenter/toolPresenter/toolPresenter.test.ts (1)
49-82: ⚡ Quick winConsider extracting a shared
agentToolRuntimefactory to reduce duplication.The
agentToolRuntimemock object (including the newly addedgenerateImageStandalone: vi.fn()) is verbatim copy-pasted across all six test cases. A single factory function or abeforeEachsetup would make future additions (like a new method onILlmProviderPresenter) a one-line change instead of six.♻️ Suggested refactor: extract a factory
+const buildAgentToolRuntime = () => ({ + resolveConversationWorkdir: vi.fn().mockResolvedValue(null), + resolveConversationSessionInfo: vi.fn().mockResolvedValue(null), + getSkillPresenter: () => + ({ + getActiveSkills: vi.fn().mockResolvedValue([]), + getActiveSkillsAllowedTools: vi.fn().mockResolvedValue([]), + listSkillScripts: vi.fn().mockResolvedValue([]), + getSkillExtension: vi.fn().mockResolvedValue({ + version: 1, + env: {}, + runtimePolicy: { python: 'auto', node: 'auto' }, + scriptOverrides: {} + }) + }) as any, + getYoBrowserToolHandler: () => ({ + getToolDefinitions: vi.fn().mockReturnValue([]), + callTool: vi.fn() + }), + getFilePresenter: () => ({ + getMimeType: vi.fn(), + prepareFileCompletely: vi.fn() + }), + getLlmProviderPresenter: () => ({ + executeWithRateLimit: vi.fn().mockResolvedValue(undefined), + generateCompletionStandalone: vi.fn(), + generateImageStandalone: vi.fn() + }), + createSettingsWindow: vi.fn(), + sendToWindow: vi.fn().mockReturnValue(true), + getApprovedFilePaths: vi.fn().mockReturnValue([]), + consumeSettingsApproval: vi.fn().mockReturnValue(false) +}) // Then each test uses: - agentToolRuntime: { - resolveConversationWorkdir: vi.fn().mockResolvedValue(null), - // ... 15 more lines ... - } + agentToolRuntime: buildAgentToolRuntime()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/main/presenter/toolPresenter/toolPresenter.test.ts` around lines 49 - 82, Extract a shared factory for the repeated mock object to remove duplication: create a helper (e.g., buildAgentToolRuntimeMock) that returns the agentToolRuntime object used in toolPresenter.test.ts and include getLlmProviderPresenter with generateImageStandalone, getYoBrowserToolHandler, getSkillPresenter, getFilePresenter, createSettingsWindow, sendToWindow, getApprovedFilePaths, consumeSettingsApproval, etc.; then replace the six inline copies with calls to buildAgentToolRuntimeMock (or set it in a beforeEach) so adding a new ILlmProviderPresenter method requires only a single edit in the factory.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/presenter/llmProviderPresenter/index.ts`:
- Around line 55-80: The createAbortPromise function must synchronously handle a
signal that is already aborted before attaching the listener: inside
createAbortPromise (and before adding the 'abort' event listener) check
signal.aborted and if true call onAbort() and return an already-rejected promise
(rejecting with createAbortError()) along with a no-op cleanup; otherwise
proceed to attach the listener as currently implemented and keep the existing
cleanup that removes the listener. Ensure you reference createAbortPromise and
the onAbort callback so callers (e.g., the stream return invocation) get invoked
immediately when signal.aborted is already true.
In `@src/renderer/src/i18n/zh-CN/settings.json`:
- Line 117: The translation key deepchatAgents.imageGenerationModel currently
uses the informal term "生图模型" which is inconsistent with
model.modelConfig.type.options.imageGeneration ("图像生成模型"); update the value of
deepchatAgents.imageGenerationModel to "图像生成模型" so both keys
(deepchatAgents.imageGenerationModel and
model.modelConfig.type.options.imageGeneration) use the same formal phrase
across zh-CN.
---
Outside diff comments:
In `@src/renderer/settings/components/DeepChatAgentsSettings.vue`:
- Around line 716-723: getGroupLabel is missing a mapping for the
'agent-image-generation' key from GROUP_ORDER, causing the raw group key to
display; update the getGroupLabel function to return a localized label for
'agent-image-generation' (using the same translation function/locale pattern as
other cases) so the UI shows a human-friendly name for that tool group.
---
Nitpick comments:
In `@test/main/presenter/toolPresenter/toolPresenter.test.ts`:
- Around line 49-82: Extract a shared factory for the repeated mock object to
remove duplication: create a helper (e.g., buildAgentToolRuntimeMock) that
returns the agentToolRuntime object used in toolPresenter.test.ts and include
getLlmProviderPresenter with generateImageStandalone, getYoBrowserToolHandler,
getSkillPresenter, getFilePresenter, createSettingsWindow, sendToWindow,
getApprovedFilePaths, consumeSettingsApproval, etc.; then replace the six inline
copies with calls to buildAgentToolRuntimeMock (or set it in a beforeEach) so
adding a new ILlmProviderPresenter method requires only a single edit in the
factory.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c71a5593-d9d9-4ddd-86ab-ddc48852da5b
📒 Files selected for processing (44)
src/main/presenter/agentRepository/index.tssrc/main/presenter/agentRuntimePresenter/dispatch.tssrc/main/presenter/agentRuntimePresenter/imageGenerationBlocks.tssrc/main/presenter/agentRuntimePresenter/index.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/toolPresenter/agentTools/agentImageGenerationTool.tssrc/main/presenter/toolPresenter/agentTools/agentToolManager.tssrc/main/presenter/toolPresenter/agentTools/index.tssrc/main/presenter/toolPresenter/index.tssrc/main/presenter/toolPresenter/runtimePorts.tssrc/renderer/settings/components/DeepChatAgentsSettings.vuesrc/renderer/src/i18n/da-DK/settings.jsonsrc/renderer/src/i18n/en-US/settings.jsonsrc/renderer/src/i18n/fa-IR/settings.jsonsrc/renderer/src/i18n/fr-FR/settings.jsonsrc/renderer/src/i18n/he-IL/settings.jsonsrc/renderer/src/i18n/ja-JP/settings.jsonsrc/renderer/src/i18n/ko-KR/settings.jsonsrc/renderer/src/i18n/pt-BR/settings.jsonsrc/renderer/src/i18n/ru-RU/settings.jsonsrc/renderer/src/i18n/zh-CN/settings.jsonsrc/renderer/src/i18n/zh-HK/settings.jsonsrc/renderer/src/i18n/zh-TW/settings.jsonsrc/shared/agentImageGenerationTool.tssrc/shared/contracts/domainSchemas.tssrc/shared/types/agent-interface.d.tssrc/shared/types/presenters/index.d.tssrc/shared/types/presenters/legacy.presenters.d.tssrc/shared/types/presenters/llmprovider.presenter.d.tstest/main/presenter/agentRepository.test.tstest/main/presenter/agentRuntimePresenter/agentRuntimePresenter.test.tstest/main/presenter/agentRuntimePresenter/dispatch.test.tstest/main/presenter/llmProviderPresenter.test.tstest/main/presenter/toolPresenter/agentTools/agentImageGenerationTool.test.tstest/main/presenter/toolPresenter/agentTools/agentToolManagerRead.test.tstest/main/presenter/toolPresenter/agentTools/agentToolManagerSettings.test.tstest/main/presenter/toolPresenter/agentTools/agentToolManagerSkillAccess.test.tstest/main/presenter/toolPresenter/agentTools/subagentOrchestratorTool.test.tstest/main/presenter/toolPresenter/toolPresenter.test.tstest/main/routes/contracts.test.tstest/renderer/components/DeepChatAgentsSettings.test.tstest/renderer/components/message/MessageBlockMedia.test.ts
Comment on lines
+55
to
+80
| const createAbortPromise = ( | ||
| signal: AbortSignal | undefined, | ||
| onAbort?: () => void | ||
| ): { promise?: Promise<never>; cleanup: () => void } => { | ||
| if (!signal) { | ||
| return { cleanup: () => undefined } | ||
| } | ||
|
|
||
| let abortHandler: (() => void) | null = null | ||
| const promise = new Promise<never>((_, reject) => { | ||
| abortHandler = () => { | ||
| onAbort?.() | ||
| reject(createAbortError()) | ||
| } | ||
| signal.addEventListener('abort', abortHandler, { once: true }) | ||
| }) | ||
|
|
||
| return { | ||
| promise, | ||
| cleanup: () => { | ||
| if (abortHandler) { | ||
| signal.removeEventListener('abort', abortHandler) | ||
| } | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '55,80p' src/main/presenter/llmProviderPresenter/index.ts
printf '\n---\n'
sed -n '382,412p' src/main/presenter/llmProviderPresenter/index.tsRepository: ThinkInAIXYZ/deepchat
Length of output: 1711
Check signal.aborted before attaching the abort listener in createAbortPromise.
The function checks for a signal and attaches an addEventListener listener, but if the signal becomes aborted between the early check at line 383 and the listener attachment at line 401, the listener will never fire because AbortSignal events only notify newly-added listeners. The code will hang on the stream operation despite the caller having cancelled.
Add a synchronous signal.aborted check inside createAbortPromise immediately after (or before) attaching the listener to handle signals that abort during the setup window:
Current code structure
// Line 383-385: Early check (insufficient)
if (signal?.aborted) {
throw createAbortError()
}
// Lines 390-397: Start stream
const stream = provider.coreStream(...)
// Lines 400-402: Attach listener (too late if signal aborts here)
const abort = createAbortPromise(signal, () => {
void stream.return?.(undefined as never)
})
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/presenter/llmProviderPresenter/index.ts` around lines 55 - 80, The
createAbortPromise function must synchronously handle a signal that is already
aborted before attaching the listener: inside createAbortPromise (and before
adding the 'abort' event listener) check signal.aborted and if true call
onAbort() and return an already-rejected promise (rejecting with
createAbortError()) along with a no-op cleanup; otherwise proceed to attach the
listener as currently implemented and keep the existing cleanup that removes the
listener. Ensure you reference createAbortPromise and the onAbort callback so
callers (e.g., the stream return invocation) get invoked immediately when
signal.aborted is already true.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/renderer/src/i18n/fr-FR/chat.json`:
- Line 52: The French locale value for the key "agentImageGeneration" is
currently English; replace the English string with a proper French translation
(for example "Génération d’images") in the fr-FR chat JSON so the UI is fully
localized; locate the "agentImageGeneration" entry in the fr-FR/chat.json and
update its value accordingly, preserving the surrounding JSON structure and
quotation style.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3ddbb6bb-bc54-423a-975a-b17d7df11fca
📒 Files selected for processing (14)
src/renderer/settings/components/DeepChatAgentsSettings.vuesrc/renderer/src/i18n/da-DK/chat.jsonsrc/renderer/src/i18n/en-US/chat.jsonsrc/renderer/src/i18n/fa-IR/chat.jsonsrc/renderer/src/i18n/fr-FR/chat.jsonsrc/renderer/src/i18n/he-IL/chat.jsonsrc/renderer/src/i18n/ja-JP/chat.jsonsrc/renderer/src/i18n/ko-KR/chat.jsonsrc/renderer/src/i18n/pt-BR/chat.jsonsrc/renderer/src/i18n/ru-RU/chat.jsonsrc/renderer/src/i18n/zh-CN/chat.jsonsrc/renderer/src/i18n/zh-HK/chat.jsonsrc/renderer/src/i18n/zh-TW/chat.jsontest/main/presenter/toolPresenter/toolPresenter.test.ts
✅ Files skipped from review due to trivial changes (11)
- src/renderer/src/i18n/en-US/chat.json
- src/renderer/src/i18n/zh-TW/chat.json
- src/renderer/src/i18n/da-DK/chat.json
- src/renderer/src/i18n/pt-BR/chat.json
- src/renderer/src/i18n/zh-HK/chat.json
- src/renderer/src/i18n/ko-KR/chat.json
- src/renderer/src/i18n/ja-JP/chat.json
- src/renderer/src/i18n/zh-CN/chat.json
- src/renderer/src/i18n/fa-IR/chat.json
- src/renderer/src/i18n/ru-RU/chat.json
- src/renderer/src/i18n/he-IL/chat.json
🚧 Files skipped from review as they are similar to previous changes (2)
- test/main/presenter/toolPresenter/toolPresenter.test.ts
- src/renderer/settings/components/DeepChatAgentsSettings.vue
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
2 participants
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.
add image generation tool
iShot_2026-05-09_09.08.32.mp4
Summary by CodeRabbit
New Features
Bug Fixes