Conversation
Contributor
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new built-in DimCode ACP agent (dimcode-acp), tracks reasoning timing metadata during streaming, updates icon resolution and UI to support the new agent, adjusts generation-setting sanitization on model switches, updates Chinese translations, and expands test coverage for these behaviors. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/presenter/deepchatAgentPresenter/index.ts`:
- Around line 620-623: When switching models the call to
sanitizeGenerationSettings is dropping session-specific overrides because it
only passes systemPrompt; change the call to pass currentGeneration (from
getEffectiveSessionGenerationSettings) as the baseSettings argument so
sanitizeGenerationSettings(nextProviderId, nextModelId, currentGeneration)
preserves temperature, context length, max tokens, and reasoning overrides while
still allowing the function to validate/merge in the new model defaults; locate
the usage at the top-level in deepchatAgentPresenter where currentGeneration and
sanitizeGenerationSettings are referenced and ensure only fields that must be
replaced are updated after sanitization.
In `@src/renderer/src/components/chat/ChatStatusBar.vue`:
- Around line 638-643: The current resolveModelIconId function returns 'acp'
when providerId === 'acp' but modelId is missing, which ModelIcon.vue doesn't
recognize; update resolveModelIconId so it only returns modelId for ACP when
modelId is present and otherwise falls back to the Anthropic icon (or the same
fallback used for other unknown providers). Specifically, change the logic in
resolveModelIconId to: if providerId === 'acp' return modelId when modelId is
truthy, otherwise return the default/fallback icon (e.g., 'anthropic');
reference resolveModelIconId, providerId, modelId and ModelIcon.vue when making
the change.
In `@test/main/presenter/deepchatAgentPresenter/deepchatAgentPresenter.test.ts`:
- Around line 1237-1242: The current cast of (agent as any).runtimeState to
Map<string, { status: string }> is too narrow for the object being set; update
the cast to use the full DeepChatSessionState shape (e.g., Map<string, { status:
string; providerId: string; modelId: string; permissionMode: string }>) so the
call to .set('s1', { status: 'generating', providerId: 'openai', modelId:
'gpt-4', permissionMode: 'full_access' }) matches the declared type and avoids
excess property/type errors when setting runtimeState on agent.
- Around line 1109-1128: The runtimeState Map is being cast too narrowly to
Map<string, { status: string }>, which omits required properties of
DeepChatSessionState; update the cast/remove the any and instead use the full
Map<string, DeepChatSessionState> type (or let TypeScript infer it) when
accessing or setting entries on (agent as any).runtimeState so the objects you
set include status, providerId, modelId, and permissionMode; adjust the two
sites that set 's1' (the current narrow-cast locations around the runtimeState
usage) to use Map<string, DeepChatSessionState> so the shape matches the
DeepChatSessionState interface.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 531cf068-bcee-4235-a998-89b339578e9b
⛔ Files ignored due to path filters (1)
src/renderer/src/assets/llm-icons/dimcode.svgis excluded by!**/*.svg
📒 Files selected for processing (22)
src/main/presenter/configPresenter/acpConfHelper.tssrc/main/presenter/configPresenter/acpInitHelper.tssrc/main/presenter/deepchatAgentPresenter/accumulator.tssrc/main/presenter/deepchatAgentPresenter/index.tssrc/renderer/src/components/chat/ChatStatusBar.vuesrc/renderer/src/components/icons/ModelIcon.vuesrc/renderer/src/i18n/zh-CN/routes.jsonsrc/renderer/src/i18n/zh-CN/settings.jsonsrc/renderer/src/i18n/zh-HK/routes.jsonsrc/renderer/src/i18n/zh-HK/settings.jsonsrc/renderer/src/i18n/zh-TW/routes.jsonsrc/renderer/src/i18n/zh-TW/settings.jsonsrc/renderer/src/pages/ChatPage.vuesrc/shared/types/agent-interface.d.tssrc/shared/types/presenters/legacy.presenters.d.tstest/main/presenter/configPresenter/acpConfHelper.test.tstest/main/presenter/deepchatAgentPresenter/accumulator.test.tstest/main/presenter/deepchatAgentPresenter/deepchatAgentPresenter.test.tstest/renderer/components/ChatPage.test.tstest/renderer/components/ChatStatusBar.test.tstest/renderer/components/ModelIcon.test.tstest/renderer/components/message/MessageBlockThink.test.ts
test/main/presenter/deepchatAgentPresenter/deepchatAgentPresenter.test.ts
Show resolved
Hide resolved
test/main/presenter/deepchatAgentPresenter/deepchatAgentPresenter.test.ts
Outdated
Show resolved
Hide resolved
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.
Summary by CodeRabbit
New Features
Behavior
Internationalization
Tests