-
Notifications
You must be signed in to change notification settings - Fork 614
Feature/add aws bedrock provider #744
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
Feature/add aws bedrock provider #744
Conversation
WalkthroughAdds AWS Bedrock as a new LLM provider: dependencies, shared types, main/provider wiring, a full provider implementation (sync and streaming), settings store credential APIs, settings UI for configuration and model management, Bedrock icon support, and i18n strings indicating Claude-only support. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Settings UI (Renderer)
participant Store as Settings Store
participant Main as Main Presenter
participant Prov as AwsBedrockProvider
participant AWS as AWS Bedrock APIs
User->>UI: Enter AWS keys/region, click Verify
UI->>Store: updateAwsBedrockProviderConfig / setAwsBedrockCredential
Store-->>UI: Updated provider config
UI->>Main: checkProvider(providerId)
Main->>Prov: init() / check()
Prov->>AWS: ListFoundationModels
AWS-->>Prov: Claude models list (or fail -> fallback)
Prov->>AWS: InvokeModel (simple prompt)
AWS-->>Prov: Response
Prov-->>Main: Check result
Main-->>UI: Status and models
UI-->>User: Show verification and model list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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
🧹 Nitpick comments (20)
src/renderer/src/components/artifacts/ArtifactDialog.vue (4)
166-166: Avoid any by typing the editor ref.Prefer a concrete element ref over any to leverage TS checks.
Example outside this hunk:
const codeEditor = ref<HTMLDivElement | null>(null)
14-15: Use English for comments and logs.Coding guidelines request English for logs and comments. The marked lines contain Chinese comments/logs; please convert to English for consistency.
Example outside this hunk:
// Top navbar console.error('Copy failed', e) // If it is a Mermaid chart, render to SVG firstAlso applies to: 331-336, 378-381
449-456: Scope component styles.Guidelines say scoped styles for Vue components. Consider adding scoped to avoid leakage.
-<style> +<style scoped>
230-236: Ensure Monaco editor is disposed on unmount.createEditor is called when the ref mounts, but I don’t see disposal on unmount. Verify vue-use-monaco handles cleanup automatically; otherwise add an onUnmounted hook to dispose to prevent leaks.
If disposal is manual, something like the following (outside this hunk) may be needed:
import { onUnmounted } from 'vue' // suppose useMonaco exposes a dispose/destroy method or returns a disposable onUnmounted(() => { // dispose editor if the helper doesn’t already })package.json (1)
60-61: Align AWS SDK client versions to reduce duplication and potential peer/version drift.Both clients can safely use the same minor to keep the monorepo in lockstep and avoid redundant copies.
Apply:
- "@aws-sdk/client-bedrock": "^3.840.0", - "@aws-sdk/client-bedrock-runtime": "^3.842.0", + "@aws-sdk/client-bedrock": "^3.842.0", + "@aws-sdk/client-bedrock-runtime": "^3.842.0",src/renderer/src/i18n/ja-JP/settings.json (1)
443-445: Polish Japanese phrasing for naturalnessCurrent text reads a bit clipped. Recommend adding verb ending and a small spacing fix.
- "bedrockLimitTip": "* Anthropic Claude(Opus、Sonnet、Haikuモデルを含む)のみをサポート" + "bedrockLimitTip": "* Anthropic Claude(Opus、Sonnet、Haiku モデル)のみをサポートします"src/renderer/src/i18n/zh-HK/settings.json (1)
443-445: Use HK-traditional wording and proper list punctuationPrefer “支援” in zh-HK locale and Chinese list separator “、”.
- "bedrockLimitTip": "* 僅支持 Anthropic Claude(包括 Opus,Sonnet,Haiku 模型)" + "bedrockLimitTip": "* 僅支援 Anthropic Claude(包括 Opus、Sonnet、Haiku 模型)"src/renderer/src/i18n/zh-CN/settings.json (1)
443-445: Use Chinese list separator for consistencyPrefer “、” for lists in Chinese punctuation.
- "bedrockLimitTip": "* 仅支持 Anthropic Claude(包括 Opus,Sonnet,Haiku 模型)" + "bedrockLimitTip": "* 仅支持 Anthropic Claude(包括 Opus、Sonnet、Haiku 模型)"src/renderer/src/i18n/fr-FR/settings.json (1)
444-444: French phrasing nit + consider centralizing shared keyCurrent phrasing is understandable but slightly awkward in French. Also, if this tip is reused across settings, consider moving it to common.json per our i18n convention.
Apply this diff to improve the French translation:
-"bedrockLimitTip": "* Prend uniquement en charge Anthropic Claude (y compris les modèles Opus, Sonnet, Haiku)" +"bedrockLimitTip": "* Ne prend en charge qu'Anthropic Claude (y compris les modèles Opus, Sonnet et Haiku)"If this tip appears in multiple sections, I suggest relocating
bedrockLimitTiptosrc/renderer/src/i18n/common.jsonand referencing it from there to avoid duplication.src/main/presenter/llmProviderPresenter/index.ts (1)
178-180: Support custom providers by apiType as wellYou special‑case
provider.id === 'aws-bedrock', which works for the built‑in provider. To support any custom provider entries that setapiType: 'bedrock', add a switch case for'bedrock'too.Apply the following change inside the
switch (provider.apiType)block:switch (provider.apiType) { + case 'bedrock': + return new AwsBedrockProvider(provider, this.configPresenter) case 'minimax': return new OpenAIProvider(provider, this.configPresenter)This keeps the explicit id branch working while enabling future Bedrock providers identified by apiType alone.
src/main/presenter/configPresenter/providers.ts (1)
595-610: Provide a helpful defaultBaseUrl hint for Bedrock
defaultBaseUrlis currently an empty string. Even if the SDK handles endpoints, the UI shows “Example API: {defaultUrl}”. A placeholder hint helps users understand the region-based endpoint.models: 'https://docs.aws.amazon.com/bedrock/latest/userguide/models-supported.html', - defaultBaseUrl: '' + defaultBaseUrl: 'https://bedrock-runtime.{region}.amazonaws.com' }Optionally, ensure the Bedrock settings UI hides the base URL field (since the SDK uses region) to avoid confusion.
src/renderer/src/components/icons/ModelIcon.vue (1)
167-169: Make invert logic easier to maintainThe invert list is growing and duplicated across includes checks. Consider centralizing invertable keys in a set to simplify future additions.
Example refactor:
- if ( - props.modelId.toLowerCase() === 'openai' || - props.modelId.toLowerCase().includes('openai-responses') || - props.modelId.toLowerCase().includes('openrouter') || - props.modelId.toLowerCase().includes('ollama') || - props.modelId.toLowerCase().includes('grok') || - props.modelId.toLowerCase().includes('groq') || - props.modelId.toLowerCase().includes('github') || - props.modelId.toLowerCase().includes('moonshot') || - props.modelId.toLowerCase().includes('lmstudio') || - props.modelId.toLowerCase().includes('aws-bedrock') - ) { + const invertKeys = ['openai', 'openai-responses', 'openrouter', 'ollama', 'grok', 'groq', 'github', 'moonshot', 'lmstudio', 'aws-bedrock'] + if (invertKeys.some(k => props.modelId.toLowerCase() === k || props.modelId.toLowerCase().includes(k))) { return true }This keeps behavior identical while reducing repetition.
src/shared/presenter.d.ts (1)
544-553: Add sessionToken to support temporary AWS credentialsMany AWS environments rely on temporary credentials (STS) requiring a session token. Adding it keeps the credential type future‑proof and matches AWS SDK usage.
export interface AwsBedrockCredential { accessKeyId: string secretAccessKey: string + /** Optional session token for temporary credentials (STS) */ + sessionToken?: string region?: string }If you add this, ensure
AwsBedrockProviderreadssessionTokenwhen constructing the AWS clients.src/renderer/src/stores/settings.ts (1)
1672-1673: Double-source of truth for credentials?You expose
setAwsBedrockCredential/getAwsBedrockCredential, but the UI updates provider.credential directly viaupdateAwsBedrockProviderConfig. Maintaining both risks divergence.If these helpers aren’t used elsewhere, consider removing them; otherwise, ensure a single source of truth (provider.credential) and keep the helpers as thin adapters to it.
src/renderer/src/components/settings/BedrockProviderSettingsDetail.vue (4)
9-16: Use appropriate placeholder and disable auto-features for accessKeyIdThe placeholder key
urlPlaceholderis misleading for credentials. Also consider disabling auto-capitalization/spellcheck.Apply this diff:
<Input :id="`${provider.id}-accessKeyId`" :model-value="accessKeyId" - :placeholder="t('settings.provider.urlPlaceholder')" + :placeholder="t('settings.provider.accessKeyIdPlaceholder')" + autocapitalize="off" + spellcheck="false" @blur="handleAccessKeyIdChange(String($event.target.value))" @keyup.enter="handleAccessKeyIdChange(accessKeyId)" @update:model-value="accessKeyId = String($event)" />Note: You’ll need to add the i18n keys if they don’t exist yet.
32-40: Region input: more specific placeholder and input hintsConsider a region-specific placeholder (e.g., “us-east-1”) and disabling spellcheck.
Apply this diff:
<Input :id="`${provider.id}-region`" :model-value="region" - :placeholder="t('settings.provider.urlPlaceholder')" + :placeholder="t('settings.provider.regionPlaceholder')" + autocapitalize="off" + spellcheck="false" @blur="handleRegionChange(String($event.target.value))" @keyup.enter="handleRegionChange(region)" @update:model-value="region = String($event)" />
202-221: Log messages should be in English and use levelsCurrent logs are mixed-language (Chinese). Prefer English for logs, and use info/warn levels consistently.
Apply this diff:
- console.log('验证成功') + console.info('AWS Bedrock credential validation succeeded') @@ - console.log('验证失败', resp.errorMsg) + console.warn('AWS Bedrock credential validation failed:', resp.errorMsg)
249-259: Typo: use 'confirm' not 'comfirm'Minor readability fix, no behavioral change.
Apply this diff:
const handleModelEnabledChange = async ( model: RENDERER_MODEL_META, enabled: boolean, - comfirm: boolean = false + confirm: boolean = false ) => { - if (!enabled && comfirm) { + if (!enabled && confirm) { disableModel(model) } else { await settingsStore.updateModelStatus(props.provider.id, model.id, enabled) } }src/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts (2)
46-50: Misleading error message when credentials are missingThis provider uses separate access keys, not a combined apiKey. Clarify the error and note region is required by the AWS SDK.
Apply this diff:
- if (!accessKeyId || !secretAccessKey) { - throw new Error( - 'apiKey must be provided as "${ACCESS_KEY_ID}:${SECRET_ACCESS_KEY}" format.' - ) - } + if (!accessKeyId || !secretAccessKey) { + throw new Error('AWS credentials missing: accessKeyId and secretAccessKey are required.') + } + if (!region) { + throw new Error('AWS region is required for Bedrock.') + }
543-548: Consider defaulting max_tokens/temperature in non-stream completions tooUnlike
coreStream,completionsmay send undefinedmax_tokens, which Bedrock can reject. Default values improve robustness.I can send a small patch mirroring the
|| 1024pattern fromcoreStreamif you’d like.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
src/renderer/src/assets/llm-icons/aws-bedrock.svgis excluded by!**/*.svg
📒 Files selected for processing (19)
package.json(1 hunks)src/main/presenter/configPresenter/providers.ts(1 hunks)src/main/presenter/llmProviderPresenter/index.ts(2 hunks)src/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts(1 hunks)src/renderer/src/components/artifacts/ArtifactDialog.vue(1 hunks)src/renderer/src/components/icons/ModelIcon.vue(3 hunks)src/renderer/src/components/settings/BedrockProviderSettingsDetail.vue(1 hunks)src/renderer/src/components/settings/ModelProviderSettings.vue(2 hunks)src/renderer/src/i18n/en-US/settings.json(1 hunks)src/renderer/src/i18n/fa-IR/settings.json(1 hunks)src/renderer/src/i18n/fr-FR/settings.json(1 hunks)src/renderer/src/i18n/ja-JP/settings.json(1 hunks)src/renderer/src/i18n/ko-KR/settings.json(1 hunks)src/renderer/src/i18n/ru-RU/settings.json(1 hunks)src/renderer/src/i18n/zh-CN/settings.json(1 hunks)src/renderer/src/i18n/zh-HK/settings.json(1 hunks)src/renderer/src/i18n/zh-TW/settings.json(1 hunks)src/renderer/src/stores/settings.ts(5 hunks)src/shared/presenter.d.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (23)
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Use English for logs and comments
Files:
src/shared/presenter.d.tssrc/main/presenter/llmProviderPresenter/index.tssrc/renderer/src/components/icons/ModelIcon.vuesrc/main/presenter/configPresenter/providers.tssrc/renderer/src/components/artifacts/ArtifactDialog.vuesrc/renderer/src/components/settings/BedrockProviderSettingsDetail.vuesrc/renderer/src/stores/settings.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.tssrc/renderer/src/components/settings/ModelProviderSettings.vue
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Strict type checking enabled for TypeScript
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪(如适用)、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/shared/presenter.d.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/configPresenter/providers.tssrc/renderer/src/stores/settings.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts
src/shared/**/*.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
Shared types in src/shared/
Files:
src/shared/presenter.d.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
Files:
src/shared/presenter.d.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/configPresenter/providers.tssrc/renderer/src/stores/settings.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts
src/shared/*.d.ts
📄 CodeRabbit Inference Engine (.cursor/rules/electron-best-practices.mdc)
The shared/*.d.ts files are used to define the types of objects exposed by the main process to the renderer process
Files:
src/shared/presenter.d.ts
src/shared/**/*.{ts,tsx,d.ts}
📄 CodeRabbit Inference Engine (.cursor/rules/project-structure.mdc)
共享类型定义放在
shared目录
Files:
src/shared/presenter.d.ts
src/main/**/*.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
Main to Renderer: Use EventBus to broadcast events via mainWindow.webContents.send()
Use Electron's built-in APIs for file system and native dialogs
Files:
src/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/configPresenter/providers.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts
src/main/presenter/**/*.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
One presenter per functional domain
Files:
src/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/configPresenter/providers.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts
src/{main,renderer}/**/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/electron-best-practices.mdc)
src/{main,renderer}/**/*.ts: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging
Files:
src/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/configPresenter/providers.tssrc/renderer/src/stores/settings.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts
src/main/presenter/llmProviderPresenter/index.ts
📄 CodeRabbit Inference Engine (.cursor/rules/llm-agent-loop.mdc)
src/main/presenter/llmProviderPresenter/index.ts:src/main/presenter/llmProviderPresenter/index.tsshould manage the overall Agent loop, conversation history, tool execution viaMcpPresenter, and frontend communication viaeventBus.
The main Agent loop inllmProviderPresenter/index.tsshould handle multi-round LLM calls and tool usage, maintaining conversation state and controlling the loop withneedContinueConversationandtoolCallCount.
The main Agent loop should send standardizedSTREAM_EVENTS(RESPONSE,END,ERROR) to the frontend viaeventBus.
The main Agent loop should buffer text content, handle tool call events, format tool results for the next LLM call, and manage conversation continuation logic.
Files:
src/main/presenter/llmProviderPresenter/index.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/configPresenter/providers.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts
src/renderer/src/**/*.vue
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/renderer/src/**/*.vue: Use Composition API for all Vue 3 components
Use Tailwind CSS with scoped styles for styling
Organize components by feature in src/renderer/src/
Follow existing component patterns in src/renderer/src/ when creating new UI components
Use Composition API with proper TypeScript typing for new UI components
Implement responsive design with Tailwind CSS for new UI components
Add proper error handling and loading states for new UI componentsUse scoped styles to prevent CSS conflicts between components
Files:
src/renderer/src/components/icons/ModelIcon.vuesrc/renderer/src/components/artifacts/ArtifactDialog.vuesrc/renderer/src/components/settings/BedrockProviderSettingsDetail.vuesrc/renderer/src/components/settings/ModelProviderSettings.vue
src/renderer/src/**/*.{ts,tsx,vue}
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/renderer/src/**/*.{ts,tsx,vue}: Use Pinia for frontend state management
Renderer to Main: Use usePresenter.ts composable for direct presenter method calls
Files:
src/renderer/src/components/icons/ModelIcon.vuesrc/renderer/src/components/artifacts/ArtifactDialog.vuesrc/renderer/src/components/settings/BedrockProviderSettingsDetail.vuesrc/renderer/src/stores/settings.tssrc/renderer/src/components/settings/ModelProviderSettings.vue
src/renderer/src/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/i18n.mdc)
src/renderer/src/**/*: All user-facing strings must use i18n keys (avoid hardcoded user-visible text in code)
Use the 'vue-i18n' framework for all internationalization in the renderer
Ensure all user-visible text in the renderer uses the translation system
Files:
src/renderer/src/components/icons/ModelIcon.vuesrc/renderer/src/components/artifacts/ArtifactDialog.vuesrc/renderer/src/components/settings/BedrockProviderSettingsDetail.vuesrc/renderer/src/i18n/en-US/settings.jsonsrc/renderer/src/i18n/ko-KR/settings.jsonsrc/renderer/src/i18n/fr-FR/settings.jsonsrc/renderer/src/i18n/zh-HK/settings.jsonsrc/renderer/src/i18n/zh-CN/settings.jsonsrc/renderer/src/i18n/ru-RU/settings.jsonsrc/renderer/src/i18n/fa-IR/settings.jsonsrc/renderer/src/i18n/zh-TW/settings.jsonsrc/renderer/src/stores/settings.tssrc/renderer/src/i18n/ja-JP/settings.jsonsrc/renderer/src/components/settings/ModelProviderSettings.vue
src/renderer/**/*.{vue,ts,js,tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/project-structure.mdc)
渲染进程代码放在
src/renderer
Files:
src/renderer/src/components/icons/ModelIcon.vuesrc/renderer/src/components/artifacts/ArtifactDialog.vuesrc/renderer/src/components/settings/BedrockProviderSettingsDetail.vuesrc/renderer/src/stores/settings.tssrc/renderer/src/components/settings/ModelProviderSettings.vue
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/vue-best-practices.mdc)
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}: Use the Composition API for better code organization and reusability
Implement proper state management with Pinia
Utilize Vue Router for navigation and route management
Leverage Vue's built-in reactivity system for efficient data handling
Files:
src/renderer/src/components/icons/ModelIcon.vuesrc/renderer/src/components/artifacts/ArtifactDialog.vuesrc/renderer/src/components/settings/BedrockProviderSettingsDetail.vuesrc/renderer/src/stores/settings.tssrc/renderer/src/components/settings/ModelProviderSettings.vue
src/renderer/**/*.{ts,tsx,vue}
📄 CodeRabbit Inference Engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,tsx,vue}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Use TypeScript for all code; prefer types over interfaces.
Avoid enums; use const objects instead.
Use arrow functions for methods and computed properties.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Files:
src/renderer/src/components/icons/ModelIcon.vuesrc/renderer/src/components/artifacts/ArtifactDialog.vuesrc/renderer/src/components/settings/BedrockProviderSettingsDetail.vuesrc/renderer/src/stores/settings.tssrc/renderer/src/components/settings/ModelProviderSettings.vue
src/renderer/**/*.{vue,ts}
📄 CodeRabbit Inference Engine (.cursor/rules/vue-shadcn.mdc)
Implement lazy loading for routes and components.
Files:
src/renderer/src/components/icons/ModelIcon.vuesrc/renderer/src/components/artifacts/ArtifactDialog.vuesrc/renderer/src/components/settings/BedrockProviderSettingsDetail.vuesrc/renderer/src/stores/settings.tssrc/renderer/src/components/settings/ModelProviderSettings.vue
src/renderer/**/*.{ts,vue}
📄 CodeRabbit Inference Engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,vue}: Use useFetch and useAsyncData for data fetching.
Implement SEO best practices using Nuxt's useHead and useSeoMeta.
Files:
src/renderer/src/components/icons/ModelIcon.vuesrc/renderer/src/components/artifacts/ArtifactDialog.vuesrc/renderer/src/components/settings/BedrockProviderSettingsDetail.vuesrc/renderer/src/stores/settings.tssrc/renderer/src/components/settings/ModelProviderSettings.vue
src/main/presenter/configPresenter/**/*.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
Centralize configuration in configPresenter/
Files:
src/main/presenter/configPresenter/providers.ts
src/main/presenter/configPresenter/providers.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
Add provider configuration in configPresenter/providers.ts when adding a new LLM provider
Files:
src/main/presenter/configPresenter/providers.ts
src/renderer/src/stores/**/*.{vue,ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/pinia-best-practices.mdc)
src/renderer/src/stores/**/*.{vue,ts,tsx,js,jsx}: Use modules to organize related state and actions
Implement proper state persistence for maintaining data across sessions
Use getters for computed state properties
Utilize actions for side effects and asynchronous operations
Keep the store focused on global state, not component-specific data
Files:
src/renderer/src/stores/settings.ts
src/main/presenter/llmProviderPresenter/providers/*.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/main/presenter/llmProviderPresenter/providers/*.ts: Create provider file in src/main/presenter/llmProviderPresenter/providers/ when adding a new LLM provider
Implement coreStream method following standardized event interface in LLM provider files
src/main/presenter/llmProviderPresenter/providers/*.ts: Each file insrc/main/presenter/llmProviderPresenter/providers/*.tsshould handle interaction with a specific LLM API, including request/response formatting, tool definition conversion, native/non-native tool call management, and standardizing output streams to a common event format.
Provider implementations must use acoreStreammethod that yields standardized stream events to decouple the main loop from provider-specific details.
ThecoreStreammethod in each Provider must perform a single streaming API request per conversation round and must not contain multi-round tool call loop logic.
Provider files should implement helper methods such asformatMessages,convertToProviderTools,parseFunctionCalls, andprepareFunctionCallPromptas needed for provider-specific logic.
All provider implementations must parse provider-specific data chunks and yield standardized events for text, reasoning, tool calls, usage, errors, stop reasons, and image data.
When a provider does not support native function calling, it must prepare messages using prompt wrapping (e.g.,prepareFunctionCallPrompt) before making the API call.
When a provider supports native function calling, MCP tools must be converted to the provider's format (e.g., usingconvertToProviderTools) and included in the API request.
Provider implementations should aggregate and yield usage events as part of the standardized stream.
Provider implementations should yield image data events in the standardized format when applicable.
Provider implementations should yield reasoning events in the standardized format when applicable.
Provider implementations should yield tool call events (`tool_call_star...
Files:
src/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts
🧠 Learnings (15)
📚 Learning: 2025-07-21T01:45:33.790Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T01:45:33.790Z
Learning: Applies to src/main/presenter/configPresenter/providers.ts : Add provider configuration in configPresenter/providers.ts when adding a new LLM provider
Applied to files:
src/shared/presenter.d.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/configPresenter/providers.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts
📚 Learning: 2025-07-21T01:45:33.790Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T01:45:33.790Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Create provider file in src/main/presenter/llmProviderPresenter/providers/ when adding a new LLM provider
Applied to files:
src/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts
📚 Learning: 2025-07-21T01:45:33.790Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T01:45:33.790Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Implement coreStream method following standardized event interface in LLM provider files
Applied to files:
src/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Each file in `src/main/presenter/llmProviderPresenter/providers/*.ts` should handle interaction with a specific LLM API, including request/response formatting, tool definition conversion, native/non-native tool call management, and standardizing output streams to a common event format.
Applied to files:
src/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Provider implementations must use a `coreStream` method that yields standardized stream events to decouple the main loop from provider-specific details.
Applied to files:
src/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Provider implementations should yield events asynchronously using the async generator pattern.
Applied to files:
src/main/presenter/llmProviderPresenter/index.ts
📚 Learning: 2025-07-23T00:45:57.322Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-07-23T00:45:57.322Z
Learning: Applies to src/renderer/**/*.{vue} : Use Iconify/Vue for icon implementation.
Applied to files:
src/renderer/src/components/icons/ModelIcon.vue
📚 Learning: 2025-07-23T00:45:57.322Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-07-23T00:45:57.322Z
Learning: Applies to src/renderer/**/*.{vue} : Use the Icon component with lucide icons.
Applied to files:
src/renderer/src/components/icons/ModelIcon.vue
📚 Learning: 2025-07-23T00:45:57.322Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-07-23T00:45:57.322Z
Learning: Applies to src/renderer/**/*.{vue} : Import Icon component from Iconify/Vue.
Applied to files:
src/renderer/src/components/icons/ModelIcon.vuesrc/renderer/src/components/settings/ModelProviderSettings.vue
📚 Learning: 2025-07-21T01:46:30.354Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/i18n.mdc:0-0
Timestamp: 2025-07-21T01:46:30.354Z
Learning: Applies to src/renderer/src/i18n/common.json : Shared translation keys must be placed in 'src/renderer/src/i18n/common.json'
Applied to files:
src/renderer/src/i18n/en-US/settings.jsonsrc/renderer/src/i18n/ko-KR/settings.jsonsrc/renderer/src/i18n/fr-FR/settings.jsonsrc/renderer/src/i18n/ru-RU/settings.jsonsrc/renderer/src/i18n/fa-IR/settings.jsonsrc/renderer/src/i18n/zh-TW/settings.json
📚 Learning: 2025-07-21T01:45:33.790Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T01:45:33.790Z
Learning: Applies to src/renderer/src/**/*.{ts,tsx,vue} : Renderer to Main: Use usePresenter.ts composable for direct presenter method calls
Applied to files:
src/renderer/src/stores/settings.ts
📚 Learning: 2025-07-21T01:45:54.229Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/electron-best-practices.mdc:0-0
Timestamp: 2025-07-21T01:45:54.229Z
Learning: Applies to src/renderer/src/composables/usePresenter.ts : The IPC in the renderer process is implemented in usePresenter.ts, allowing direct calls to the presenter-related interfaces exposed by the main process
Applied to files:
src/renderer/src/stores/settings.ts
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/index.ts : `src/main/presenter/llmProviderPresenter/index.ts` should manage the overall Agent loop, conversation history, tool execution via `McpPresenter`, and frontend communication via `eventBus`.
Applied to files:
src/renderer/src/stores/settings.ts
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Provider files should implement helper methods such as `formatMessages`, `convertToProviderTools`, `parseFunctionCalls`, and `prepareFunctionCallPrompt` as needed for provider-specific logic.
Applied to files:
src/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : All provider implementations must parse provider-specific data chunks and yield standardized events for text, reasoning, tool calls, usage, errors, stop reasons, and image data.
Applied to files:
src/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts
🧬 Code Graph Analysis (3)
src/main/presenter/llmProviderPresenter/index.ts (1)
src/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts (1)
AwsBedrockProvider(23-918)
src/renderer/src/stores/settings.ts (1)
src/shared/presenter.d.ts (2)
AWS_BEDROCK_PROVIDER(544-546)AwsBedrockCredential(548-552)
src/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts (3)
src/shared/presenter.d.ts (5)
AWS_BEDROCK_PROVIDER(544-546)MODEL_META(480-493)ChatMessage(1264-1276)LLMResponse(836-853)LLMCoreStreamEvent(1223-1261)src/main/presenter/llmProviderPresenter/baseProvider.ts (1)
SUMMARY_TITLES_PROMPT(678-680)src/main/presenter/index.ts (1)
presenter(184-184)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-check (x64)
🔇 Additional comments (14)
src/renderer/src/components/artifacts/ArtifactDialog.vue (1)
165-165: Confirm useMonaco MAX_HEIGHT value type (number vs. CSS string).You switched MAX_HEIGHT from '500px' to 500. Please confirm vue-use-monaco expects a number in px and not a CSS string; otherwise height handling may break.
If the API expects a CSS string, revert to:
-const { createEditor, updateCode } = useMonaco({ MAX_HEIGHT: 500 }) +const { createEditor, updateCode } = useMonaco({ MAX_HEIGHT: '500px' })src/renderer/src/i18n/en-US/settings.json (1)
443-445: Locale parity and UI integration confirmed for provider.bedrockLimitTip
- ✅ Present in all locales under
src/renderer/src/i18n/*/settings.json- ✅ Referenced at
src/renderer/src/components/settings/BedrockProviderSettingsDetail.vue:57No further changes needed—this key is available across locales and displayed in the UI as intended.
src/renderer/src/i18n/zh-TW/settings.json (1)
443-445: Locale entry added correctly; verify parity.The zh-TW bedrockLimitTip aligns with en-US. Please confirm all locales include this key and UI references are wired.
Use the same script shared in the en-US comment to verify.
src/renderer/src/i18n/ru-RU/settings.json (1)
443-445: RU translation looks consistent; verify parity and wiring.Text is consistent with other locales. Ensure the key exists across locales and is used in the settings UI.
Use the verification script from the en-US comment to check presence and usage.
src/renderer/src/i18n/ko-KR/settings.json (1)
443-445: LGTMTranslation reads naturally and JSON structure is valid. No issues.
src/renderer/src/i18n/fr-FR/settings.json (1)
443-443: LGTM on trailing comma additionNo issues with the trailing comma to accommodate the new key.
src/main/presenter/llmProviderPresenter/index.ts (1)
29-29: LGTM: AwsBedrockProvider importImport is correct and consistent with existing provider imports.
src/renderer/src/components/icons/ModelIcon.vue (2)
61-61: LGTM: AWS Bedrock icon importedImport path and usage align with existing icon conventions.
128-130: LGTM: Map key for 'aws-bedrock'The mapping exposes the icon via a stable key that matches the provider id.
src/renderer/src/components/settings/ModelProviderSettings.vue (3)
140-145: Bedrock settings branch wiring looks correctThe conditional, keying, and typed cast to AWS_BEDROCK_PROVIDER align with the existing pattern (Ollama/Anthropic). No concerns.
167-167: Import of BedrockProviderSettingsDetail is consistent with component usageImport path and usage match the new Bedrock detail component. Good to go.
172-172: Type import for AWS_BEDROCK_PROVIDER is appropriateThe cast resolves prop typing for the Bedrock settings component without affecting the rest of the component’s types. LGTM.
src/renderer/src/stores/settings.ts (1)
8-8: Type imports extended correctlyAdding AWS_BEDROCK_PROVIDER and AwsBedrockCredential here is consistent with new Bedrock support; no issues.
src/renderer/src/components/settings/BedrockProviderSettingsDetail.vue (1)
73-88: Confirm whether check-result is used by ProviderDialogContainer
checkResultis never set (assignments commented out), yet it’s passed to ProviderDialogContainer. If the dialog uses it to show success/failure, results may be misleading.Would you like me to wire
checkResultback up (set on success/failure) or remove the prop if it’s unused?
| private defaultModel = 'claude-3-7-sonnet-20250219' | ||
|
|
||
| constructor(provider: AWS_BEDROCK_PROVIDER, configPresenter: ConfigPresenter) { | ||
| super(provider, configPresenter) | ||
| this.init() | ||
| } |
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.
Incorrect default model id for Bedrock (will fail check calls)
claude-3-7-sonnet-20250219 is not a valid Bedrock modelId. Bedrock model IDs for Claude carry the anthropic. prefix and version suffix, e.g. anthropic.claude-3-7-sonnet-20250219-v1:0. The current value will cause check() to 404 even with valid credentials.
Apply this diff:
- private defaultModel = 'claude-3-7-sonnet-20250219'
+ private defaultModel = 'anthropic.claude-3-7-sonnet-20250219-v1:0'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private defaultModel = 'claude-3-7-sonnet-20250219' | |
| constructor(provider: AWS_BEDROCK_PROVIDER, configPresenter: ConfigPresenter) { | |
| super(provider, configPresenter) | |
| this.init() | |
| } | |
| private defaultModel = 'anthropic.claude-3-7-sonnet-20250219-v1:0' | |
| constructor(provider: AWS_BEDROCK_PROVIDER, configPresenter: ConfigPresenter) { | |
| super(provider, configPresenter) | |
| this.init() | |
| } |
🤖 Prompt for AI Agents
In src/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts
around lines 26 to 31, the defaultModel value is an invalid Bedrock model id and
will cause check() to 404; update the defaultModel to a valid Bedrock model
identifier (for Claude models include the "anthropic." prefix and
version/suffix), e.g. set it to "anthropic.claude-3-7-sonnet-20250219-v1:0" or
another confirmed Bedrock model id, and ensure the value is used consistently in
check() and any validation so the provider can successfully call Bedrock.
| protected async fetchProviderModels(): Promise<MODEL_META[]> { | ||
| try { | ||
| const command = new ListFoundationModelsCommand({}) | ||
| const response = await this.bedrock.send(command) | ||
| const models = response.modelSummaries | ||
|
|
||
| return ( | ||
| models | ||
| ?.filter((m) => m.modelId?.includes('anthropic.claude')) | ||
| ?.filter((m) => m.modelLifecycle?.status === 'ACTIVE') | ||
| .map<MODEL_META>((m) => ({ | ||
| // id: m.modelId!, | ||
| // id: m.modelArn!, | ||
| id: `${m.inferenceTypesSupported?.includes('ON_DEMAND') ? m.modelId! : `us.${m.modelId}`}`, | ||
| // name: `${m.modelName} (${m.modelId})`, | ||
| name: m.modelId?.replace('anthropic.', '') || '<Unknown>', | ||
| providerId: this.provider.id, | ||
| maxTokens: 64_000, | ||
| group: `AWS Bedrock Claude - ${m.modelId?.includes('opus') ? 'opus' : m.modelId?.includes('sonnet') ? 'sonnet' : m.modelId?.includes('haiku') ? 'haiku' : 'other'}`, | ||
| isCustom: false, | ||
| contextLength: 200_000, | ||
| vision: false, | ||
| functionCall: false, | ||
| reasoning: false | ||
| })) || [] | ||
| ) | ||
| } catch (error) { | ||
| console.error('获取AWS Bedrock Anthropic模型列表出错:', error) | ||
| } | ||
|
|
||
| // 如果API请求失败或返回数据解析失败,返回默认模型列表 |
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.
Model id mapping likely breaks inference (unexpected 'us.' prefix)
Returning id: us.${m.modelId} when ON_DEMAND isn’t supported is not a Bedrock modelId and will cause runtime errors when used. You should return the actual modelId Bedrock expects. Region is provided via client config, not by prefixing the id.
Apply this diff:
- id: `${m.inferenceTypesSupported?.includes('ON_DEMAND') ? m.modelId! : `us.${m.modelId}`}`,
+ id: m.modelId!,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| protected async fetchProviderModels(): Promise<MODEL_META[]> { | |
| try { | |
| const command = new ListFoundationModelsCommand({}) | |
| const response = await this.bedrock.send(command) | |
| const models = response.modelSummaries | |
| return ( | |
| models | |
| ?.filter((m) => m.modelId?.includes('anthropic.claude')) | |
| ?.filter((m) => m.modelLifecycle?.status === 'ACTIVE') | |
| .map<MODEL_META>((m) => ({ | |
| // id: m.modelId!, | |
| // id: m.modelArn!, | |
| id: `${m.inferenceTypesSupported?.includes('ON_DEMAND') ? m.modelId! : `us.${m.modelId}`}`, | |
| // name: `${m.modelName} (${m.modelId})`, | |
| name: m.modelId?.replace('anthropic.', '') || '<Unknown>', | |
| providerId: this.provider.id, | |
| maxTokens: 64_000, | |
| group: `AWS Bedrock Claude - ${m.modelId?.includes('opus') ? 'opus' : m.modelId?.includes('sonnet') ? 'sonnet' : m.modelId?.includes('haiku') ? 'haiku' : 'other'}`, | |
| isCustom: false, | |
| contextLength: 200_000, | |
| vision: false, | |
| functionCall: false, | |
| reasoning: false | |
| })) || [] | |
| ) | |
| } catch (error) { | |
| console.error('获取AWS Bedrock Anthropic模型列表出错:', error) | |
| } | |
| // 如果API请求失败或返回数据解析失败,返回默认模型列表 | |
| protected async fetchProviderModels(): Promise<MODEL_META[]> { | |
| try { | |
| const command = new ListFoundationModelsCommand({}) | |
| const response = await this.bedrock.send(command) | |
| const models = response.modelSummaries | |
| return ( | |
| models | |
| ?.filter((m) => m.modelId?.includes('anthropic.claude')) | |
| ?.filter((m) => m.modelLifecycle?.status === 'ACTIVE') | |
| .map<MODEL_META>((m) => ({ | |
| // id: m.modelId!, | |
| // id: m.modelArn!, | |
| id: m.modelId!, | |
| // name: `${m.modelName} (${m.modelId})`, | |
| name: m.modelId?.replace('anthropic.', '') || '<Unknown>', | |
| providerId: this.provider.id, | |
| maxTokens: 64_000, | |
| group: `AWS Bedrock Claude - ${m.modelId?.includes('opus') ? 'opus' : m.modelId?.includes('sonnet') ? 'sonnet' : m.modelId?.includes('haiku') ? 'haiku' : 'other'}`, | |
| isCustom: false, | |
| contextLength: 200_000, | |
| vision: false, | |
| functionCall: false, | |
| reasoning: false | |
| })) || [] | |
| ) | |
| } catch (error) { | |
| console.error('获取AWS Bedrock Anthropic模型列表出错:', error) | |
| } | |
| // 如果API请求失败或返回数据解析失败,返回默认模型列表 | |
| } |
🤖 Prompt for AI Agents
In src/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts
around lines 69 to 99, the current id mapping prefixes non-ON_DEMAND models with
"us." which is not a valid Bedrock modelId and breaks inference; change the
mapping to return the actual Bedrock model identifier (use m.modelId or
m.modelArn as the canonical id) instead of adding a region prefix, e.g. set id
to m.modelId! (with a safe fallback if necessary), and remove the conditional
prefix logic so the client uses the model id configured by Bedrock rather than a
fabricated one.
| const payload = { | ||
| anthropic_version: 'bedrock-2023-05-31', | ||
| max_tokens: maxTokens, | ||
| temperature, | ||
| system: formattedMessages.system, | ||
| messages | ||
| } |
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.
Payload uses unformatted ChatMessage objects (Bedrock will reject it)
You compute formattedMessages but don’t use it in the payload. This sends the wrong shape and will fail.
Apply this diff:
const payload = {
anthropic_version: 'bedrock-2023-05-31',
max_tokens: maxTokens,
temperature,
- system: formattedMessages.system,
- messages
+ system: formattedMessages.system,
+ messages: formattedMessages.messages
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const payload = { | |
| anthropic_version: 'bedrock-2023-05-31', | |
| max_tokens: maxTokens, | |
| temperature, | |
| system: formattedMessages.system, | |
| messages | |
| } | |
| const payload = { | |
| anthropic_version: 'bedrock-2023-05-31', | |
| max_tokens: maxTokens, | |
| temperature, | |
| system: formattedMessages.system, | |
| messages: formattedMessages.messages | |
| } |
🤖 Prompt for AI Agents
In src/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts
around lines 557 to 563, the payload is being built using the local unformatted
messages variable rather than the formattedMessages you computed; update the
payload to use formattedMessages.messages (and keep formattedMessages.system) so
the payload shape matches Bedrock's expected format and won’t be rejected.
| console.log('modelConfig', modelConfig, modelId) | ||
| try { | ||
| // 格式化消息 | ||
| const formattedMessagesObject = this.formatMessages(messages) | ||
| console.log('formattedMessagesObject', JSON.stringify(formattedMessagesObject)) | ||
|
|
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.
🛠️ Refactor suggestion
Avoid logging full formatted messages (PII risk)
Dumping message payloads into logs can leak sensitive user data. Remove or guard behind a debug flag.
Apply this diff:
- console.log('formattedMessagesObject', JSON.stringify(formattedMessagesObject))
+ // Consider logging only metadata in DEBUG mode to avoid leaking sensitive content📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.log('modelConfig', modelConfig, modelId) | |
| try { | |
| // 格式化消息 | |
| const formattedMessagesObject = this.formatMessages(messages) | |
| console.log('formattedMessagesObject', JSON.stringify(formattedMessagesObject)) | |
| console.log('modelConfig', modelConfig, modelId) | |
| try { | |
| // 格式化消息 | |
| const formattedMessagesObject = this.formatMessages(messages) | |
| // Consider logging only metadata in DEBUG mode to avoid leaking sensitive content |
🤖 Prompt for AI Agents
In src/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts
around lines 640 to 645, the code logs full formatted messages and modelConfig
(risking PII exposure); remove these unconditional console.log calls and either
delete them or wrap them behind an explicit debug flag or logger-level check
(e.g., if (this.logger.isDebugEnabled()) or a process.env.DEBUG check) so they
only run in debug mode, and when logging messages prefer redacting sensitive
fields or logging metadata only (message count, roles, modelId) instead of the
full payload.
| const payload = { | ||
| anthropic_version: 'bedrock-2023-05-31', | ||
| max_tokens: maxTokens || 1024, | ||
| temperature: temperature || 0.7, | ||
| // system: formattedMessagesObject.system, | ||
| messages: formattedMessagesObject.messages, | ||
| thinking: undefined as any, | ||
| tools: undefined as any | ||
| } | ||
| const command = new InvokeModelWithResponseStreamCommand({ | ||
| contentType: 'application/json', | ||
| body: JSON.stringify(payload), | ||
| modelId | ||
| }) | ||
|
|
||
| // 启用Claude 3.7模型的思考功能 | ||
| if (modelId.includes('claude-3-7')) { | ||
| payload.thinking = { budget_tokens: 1024, type: 'enabled' } | ||
| } | ||
|
|
||
| // // 如果有系统消息,添加到请求参数中 | ||
| // if (formattedMessagesObject.system) { | ||
| // // @ts-ignore - system属性在类型定义中可能不存在,但API已支持 | ||
| // streamParams.system = formattedMessagesObject.system | ||
| // } | ||
|
|
||
| // 添加工具参数 | ||
| if (anthropicTools && anthropicTools.length > 0) { | ||
| // @ts-ignore - 类型不匹配,但格式是正确的 | ||
| payload.tools = anthropicTools | ||
| } | ||
| // 创建Anthropic流 |
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.
Stream request builds command before finalizing payload (thinking/tools/system not included)
You create InvokeModelWithResponseStreamCommand before setting thinking, tools, and system, so the request body lacks these fields.
Build the payload fully (including system, tools, thinking) before constructing the command.
Apply this diff:
- const payload = {
- anthropic_version: 'bedrock-2023-05-31',
- max_tokens: maxTokens || 1024,
- temperature: temperature || 0.7,
- // system: formattedMessagesObject.system,
- messages: formattedMessagesObject.messages,
- thinking: undefined as any,
- tools: undefined as any
- }
- const command = new InvokeModelWithResponseStreamCommand({
- contentType: 'application/json',
- body: JSON.stringify(payload),
- modelId
- })
+ const payload: any = {
+ anthropic_version: 'bedrock-2023-05-31',
+ max_tokens: maxTokens || 1024,
+ temperature: temperature || 0.7,
+ system: formattedMessagesObject.system,
+ messages: formattedMessagesObject.messages
+ }
@@
if (anthropicTools && anthropicTools.length > 0) {
// @ts-ignore - 类型不匹配,但格式是正确的
- payload.tools = anthropicTools
+ payload.tools = anthropicTools
}
+ const command = new InvokeModelWithResponseStreamCommand({
+ contentType: 'application/json',
+ body: JSON.stringify(payload),
+ modelId
+ })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const payload = { | |
| anthropic_version: 'bedrock-2023-05-31', | |
| max_tokens: maxTokens || 1024, | |
| temperature: temperature || 0.7, | |
| // system: formattedMessagesObject.system, | |
| messages: formattedMessagesObject.messages, | |
| thinking: undefined as any, | |
| tools: undefined as any | |
| } | |
| const command = new InvokeModelWithResponseStreamCommand({ | |
| contentType: 'application/json', | |
| body: JSON.stringify(payload), | |
| modelId | |
| }) | |
| // 启用Claude 3.7模型的思考功能 | |
| if (modelId.includes('claude-3-7')) { | |
| payload.thinking = { budget_tokens: 1024, type: 'enabled' } | |
| } | |
| // // 如果有系统消息,添加到请求参数中 | |
| // if (formattedMessagesObject.system) { | |
| // // @ts-ignore - system属性在类型定义中可能不存在,但API已支持 | |
| // streamParams.system = formattedMessagesObject.system | |
| // } | |
| // 添加工具参数 | |
| if (anthropicTools && anthropicTools.length > 0) { | |
| // @ts-ignore - 类型不匹配,但格式是正确的 | |
| payload.tools = anthropicTools | |
| } | |
| // 创建Anthropic流 | |
| const payload: any = { | |
| anthropic_version: 'bedrock-2023-05-31', | |
| max_tokens: maxTokens || 1024, | |
| temperature: temperature || 0.7, | |
| system: formattedMessagesObject.system, | |
| messages: formattedMessagesObject.messages | |
| } | |
| // 启用Claude 3.7模型的思考功能 | |
| if (modelId.includes('claude-3-7')) { | |
| payload.thinking = { budget_tokens: 1024, type: 'enabled' } | |
| } | |
| // // 如果有系统消息,添加到请求参数中 | |
| // if (formattedMessagesObject.system) { | |
| // // @ts-ignore - system属性在类型定义中可能不存在,但API已支持 | |
| // streamParams.system = formattedMessagesObject.system | |
| // } | |
| // 添加工具参数 | |
| if (anthropicTools && anthropicTools.length > 0) { | |
| // @ts-ignore - 类型不匹配,但格式是正确的 | |
| payload.tools = anthropicTools | |
| } | |
| const command = new InvokeModelWithResponseStreamCommand({ | |
| contentType: 'application/json', | |
| body: JSON.stringify(payload), | |
| modelId | |
| }) | |
| // 创建Anthropic流 |
🤖 Prompt for AI Agents
In src/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts
around lines 660 to 691, the InvokeModelWithResponseStreamCommand is constructed
before the payload is finalized so thinking, tools and system fields are
omitted; move the command construction to after you set payload.thinking,
payload.tools and include formattedMessagesObject.system (if present) into
payload, then JSON.stringify that completed payload into the command body;
ensure any type-ignores remain and only create the command once the payload is
fully built.
| const handleSecretAccessKeyChange = (value: string) => { | ||
| settingsStore.updateAwsBedrockProviderConfig(props.provider.id, { | ||
| credential: { | ||
| accessKeyId: accessKeyId.value, | ||
| secretAccessKey: value, | ||
| region: region.value | ||
| } | ||
| }) | ||
| } |
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.
🛠️ Refactor suggestion
Ditto: await and handle errors for secret update
Apply this diff:
-const handleSecretAccessKeyChange = (value: string) => {
- settingsStore.updateAwsBedrockProviderConfig(props.provider.id, {
+const handleSecretAccessKeyChange = async (value: string) => {
+ try {
+ await settingsStore.updateAwsBedrockProviderConfig(props.provider.id, {
credential: {
accessKeyId: accessKeyId.value,
secretAccessKey: value,
region: region.value
}
- })
+ })
+ } catch (err) {
+ console.error('Failed to update secretAccessKey:', err)
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleSecretAccessKeyChange = (value: string) => { | |
| settingsStore.updateAwsBedrockProviderConfig(props.provider.id, { | |
| credential: { | |
| accessKeyId: accessKeyId.value, | |
| secretAccessKey: value, | |
| region: region.value | |
| } | |
| }) | |
| } | |
| const handleSecretAccessKeyChange = async (value: string) => { | |
| try { | |
| await settingsStore.updateAwsBedrockProviderConfig(props.provider.id, { | |
| credential: { | |
| accessKeyId: accessKeyId.value, | |
| secretAccessKey: value, | |
| region: region.value | |
| } | |
| }) | |
| } catch (err) { | |
| console.error('Failed to update secretAccessKey:', err) | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/renderer/src/components/settings/BedrockProviderSettingsDetail.vue around
lines 182 to 190, the secret-access-key update is called without awaiting or
handling failures; make the handler async, await the
settingsStore.updateAwsBedrockProviderConfig call, wrap it in a try/catch, and
on error log the exception and surface a user-facing error (e.g., via the
existing notification/toast mechanism) so failures are not swallowed and the UI
can react appropriately.
| const handleRegionChange = (value: string) => { | ||
| settingsStore.updateAwsBedrockProviderConfig(props.provider.id, { | ||
| credential: { | ||
| accessKeyId: accessKeyId.value, | ||
| secretAccessKey: secretAccessKey.value, | ||
| region: value || undefined | ||
| } | ||
| }) | ||
| } |
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.
🛠️ Refactor suggestion
Ditto: await and handle errors for region update
Apply this diff:
-const handleRegionChange = (value: string) => {
- settingsStore.updateAwsBedrockProviderConfig(props.provider.id, {
+const handleRegionChange = async (value: string) => {
+ try {
+ await settingsStore.updateAwsBedrockProviderConfig(props.provider.id, {
credential: {
accessKeyId: accessKeyId.value,
secretAccessKey: secretAccessKey.value,
region: value || undefined
}
- })
+ })
+ } catch (err) {
+ console.error('Failed to update region:', err)
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleRegionChange = (value: string) => { | |
| settingsStore.updateAwsBedrockProviderConfig(props.provider.id, { | |
| credential: { | |
| accessKeyId: accessKeyId.value, | |
| secretAccessKey: secretAccessKey.value, | |
| region: value || undefined | |
| } | |
| }) | |
| } | |
| const handleRegionChange = async (value: string) => { | |
| try { | |
| await settingsStore.updateAwsBedrockProviderConfig(props.provider.id, { | |
| credential: { | |
| accessKeyId: accessKeyId.value, | |
| secretAccessKey: secretAccessKey.value, | |
| region: value || undefined | |
| } | |
| }) | |
| } catch (err) { | |
| console.error('Failed to update region:', err) | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/renderer/src/components/settings/BedrockProviderSettingsDetail.vue around
lines 192 to 200, the region update call is not awaited and lacks error
handling; make handleRegionChange async, await
settingsStore.updateAwsBedrockProviderConfig(...) and wrap the await in a
try/catch block so errors are caught; on success optionally show or clear a
success state, and on catch log the error and surface a user-facing notification
(or set an error reactive state) so failures are visible and do not silently
fail.
| // 更新 AWS Bedrock Provider 的配置 | ||
| const updateAwsBedrockProviderConfig = async ( | ||
| providerId: string, | ||
| updates: Partial<AWS_BEDROCK_PROVIDER> | ||
| ): Promise<void> => { | ||
| await updateProviderConfig(providerId, updates) | ||
| const currentProvider = providers.value.find((p) => p.id === providerId)! | ||
|
|
||
| // 只在特定条件下刷新模型列表 | ||
| const needRefreshModels = ['accessKeyId', 'secretAccessKey', 'region'].some( | ||
| (key) => key in updates | ||
| ) | ||
| if (needRefreshModels && currentProvider.enable) { | ||
| await refreshAllModels() | ||
| } | ||
| } | ||
|
|
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.
Credential-change detection does not trigger model refresh (nested keys under credential are ignored)
updateAwsBedrockProviderConfig checks top-level keys accessKeyId/secretAccessKey/region. However, callers pass { credential: { ... } }. This means needRefreshModels is always false and models won’t refresh after credential changes. Also, the non-null assertion on currentProvider can throw.
Update to detect nested changes safely and guard when provider not found.
Apply this diff:
const updateAwsBedrockProviderConfig = async (
providerId: string,
updates: Partial<AWS_BEDROCK_PROVIDER>
): Promise<void> => {
- await updateProviderConfig(providerId, updates)
- const currentProvider = providers.value.find((p) => p.id === providerId)!
-
- // 只在特定条件下刷新模型列表
- const needRefreshModels = ['accessKeyId', 'secretAccessKey', 'region'].some(
- (key) => key in updates
- )
- if (needRefreshModels && currentProvider.enable) {
- await refreshAllModels()
- }
+ await updateProviderConfig(providerId, updates)
+ const currentProvider = providers.value.find((p) => p.id === providerId)
+ if (!currentProvider) return
+
+ // Refresh when credential fields were provided (nested under updates.credential)
+ const cred = (updates as Partial<AWS_BEDROCK_PROVIDER>).credential
+ const needRefreshModels =
+ !!cred &&
+ (typeof cred.accessKeyId !== 'undefined' ||
+ typeof cred.secretAccessKey !== 'undefined' ||
+ typeof cred.region !== 'undefined')
+ if (needRefreshModels && currentProvider.enable) {
+ await refreshAllModels()
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 更新 AWS Bedrock Provider 的配置 | |
| const updateAwsBedrockProviderConfig = async ( | |
| providerId: string, | |
| updates: Partial<AWS_BEDROCK_PROVIDER> | |
| ): Promise<void> => { | |
| await updateProviderConfig(providerId, updates) | |
| const currentProvider = providers.value.find((p) => p.id === providerId)! | |
| // 只在特定条件下刷新模型列表 | |
| const needRefreshModels = ['accessKeyId', 'secretAccessKey', 'region'].some( | |
| (key) => key in updates | |
| ) | |
| if (needRefreshModels && currentProvider.enable) { | |
| await refreshAllModels() | |
| } | |
| } | |
| // 更新 AWS Bedrock Provider 的配置 | |
| const updateAwsBedrockProviderConfig = async ( | |
| providerId: string, | |
| updates: Partial<AWS_BEDROCK_PROVIDER> | |
| ): Promise<void> => { | |
| await updateProviderConfig(providerId, updates) | |
| const currentProvider = providers.value.find((p) => p.id === providerId) | |
| if (!currentProvider) return | |
| // Refresh when credential fields were provided (nested under updates.credential) | |
| const cred = (updates as Partial<AWS_BEDROCK_PROVIDER>).credential | |
| const needRefreshModels = | |
| !!cred && | |
| (typeof cred.accessKeyId !== 'undefined' || | |
| typeof cred.secretAccessKey !== 'undefined' || | |
| typeof cred.region !== 'undefined') | |
| if (needRefreshModels && currentProvider.enable) { | |
| await refreshAllModels() | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/renderer/src/stores/settings.ts around lines 865 to 881,
updateAwsBedrockProviderConfig currently checks only top-level keys
('accessKeyId','secretAccessKey','region') so updates like { credential: {
accessKeyId: ... } } won't trigger a model refresh and the non-null assertion on
currentProvider can throw; change the logic to safely find the provider (return
early or throw if not found instead of using '!'), and determine
needRefreshModels by checking both top-level keys and nested keys under
updates.credential (e.g., if updates.credential exists and any of the credential
keys are present) or updates.region at top-level, then call refreshAllModels()
only when the provider exists and is enabled.
| // AWS Bedrock | ||
| const setAwsBedrockCredential = async (credential: AwsBedrockCredential) => { | ||
| await configP.setSetting('awsBedrockCredential', JSON.stringify({ credential })) | ||
| } | ||
|
|
||
| const getAwsBedrockCredential = async (): Promise<AwsBedrockCredential | undefined> => { | ||
| return await configP.getSetting<AwsBedrockCredential | undefined>('awsBedrockCredential') | ||
| } | ||
|
|
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.
Bug: Credential is saved as JSON string but read as an object
setAwsBedrockCredential stores a JSON string, while getAwsBedrockCredential reads the raw value as an object. This will break retrieval and can leak implementation detail to callers.
Store an object; when reading, accept both object and legacy string (for compatibility).
Apply this diff:
// AWS Bedrock
const setAwsBedrockCredential = async (credential: AwsBedrockCredential) => {
- await configP.setSetting('awsBedrockCredential', JSON.stringify({ credential }))
+ await configP.setSetting('awsBedrockCredential', { credential })
}
const getAwsBedrockCredential = async (): Promise<AwsBedrockCredential | undefined> => {
- return await configP.getSetting<AwsBedrockCredential | undefined>('awsBedrockCredential')
+ const raw = await configP.getSetting<unknown>('awsBedrockCredential')
+ if (!raw) return undefined
+ // Backward compatibility: handle legacy string value
+ if (typeof raw === 'string') {
+ try {
+ const parsed = JSON.parse(raw)
+ return parsed?.credential as AwsBedrockCredential | undefined
+ } catch {
+ return undefined
+ }
+ }
+ // Preferred shape: { credential: AwsBedrockCredential }
+ const obj = raw as { credential?: AwsBedrockCredential }
+ return obj.credential
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // AWS Bedrock | |
| const setAwsBedrockCredential = async (credential: AwsBedrockCredential) => { | |
| await configP.setSetting('awsBedrockCredential', JSON.stringify({ credential })) | |
| } | |
| const getAwsBedrockCredential = async (): Promise<AwsBedrockCredential | undefined> => { | |
| return await configP.getSetting<AwsBedrockCredential | undefined>('awsBedrockCredential') | |
| } | |
| // AWS Bedrock | |
| const setAwsBedrockCredential = async (credential: AwsBedrockCredential) => { | |
| await configP.setSetting('awsBedrockCredential', { credential }) | |
| } | |
| const getAwsBedrockCredential = async (): Promise<AwsBedrockCredential | undefined> => { | |
| const raw = await configP.getSetting<unknown>('awsBedrockCredential') | |
| if (!raw) return undefined | |
| // Backward compatibility: handle legacy string value | |
| if (typeof raw === 'string') { | |
| try { | |
| const parsed = JSON.parse(raw) | |
| return parsed?.credential as AwsBedrockCredential | undefined | |
| } catch { | |
| return undefined | |
| } | |
| } | |
| // Preferred shape: { credential: AwsBedrockCredential } | |
| const obj = raw as { credential?: AwsBedrockCredential } | |
| return obj.credential | |
| } |
🤖 Prompt for AI Agents
In src/renderer/src/stores/settings.ts around lines 1557 to 1565,
setAwsBedrockCredential currently saves a JSON string while
getAwsBedrockCredential expects an object; change setAwsBedrockCredential to
save the credential object directly (remove JSON.stringify) and change
getAwsBedrockCredential to accept both legacy string and object values by
detecting a string, parsing it (try/catch) to return the AwsBedrockCredential,
and otherwise returning the stored object; keep compatibility by returning
undefined on parse failure or missing value.
|
LGTM |
Pull Request Description
Is your feature request related to a problem? Please describe.
Add new provider: AWS Bedrock (only Claude supported)
Pull Request Description (中文)
你的功能请求是否与某个问题有关?请描述一下。
新增provider:AWS Bedrock(仅支持 Claude)
Summary by CodeRabbit