-
Notifications
You must be signed in to change notification settings - Fork 614
refactor(provider-db): migrate model config to external DB #956
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
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a Provider DB pipeline: a prebuild fetch-and-sanitize script, typed schemas and sanitizer, a ProviderDbLoader with cache/TTL/etag refresh and events, ConfigPresenter DB exposure and DB-driven model/config resolution, provider fallbacks switched to DB-derived lists, renderer listeners, and related tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App as Main
participant Loader as ProviderDbLoader
participant Disk as Cache (resources/model-db)
participant Net as Provider DB HTTP
participant Bus as eventBus
participant UI as Renderer
User->>App: start
App->>Loader: initialize()
Loader->>Disk: try loadFromCache()
alt cache found
Loader--)Bus: emit PROVIDER_DB_EVENTS.LOADED
else no cache
Loader->>Net: fetch (If-None-Match)
alt 200 OK
Loader->>Loader: sanitizeAggregate()
Loader->>Disk: atomic write cache + meta
Loader--)Bus: emit PROVIDER_DB_EVENTS.LOADED
else 304
Loader->>Disk: update meta
Loader--)Bus: emit PROVIDER_DB_EVENTS.LOADED
else error
Loader->>Disk: fallback to existing snapshot (if any)
Loader--)Bus: emit PROVIDER_DB_EVENTS.LOADED (if snapshot)
end
end
Bus--)UI: PROVIDER_DB_EVENTS.LOADED -> refreshAllModels()
rect rgba(220,240,230,0.25)
App->>Loader: periodic refreshIfNeeded()
Loader->>Net: fetch with ETag
alt updated
Loader->>Disk: atomic update
Loader--)Bus: emit PROVIDER_DB_EVENTS.UPDATED
Bus--)UI: UPDATED -> refreshAllModels()
end
end
sequenceDiagram
autonumber
participant Provider as ProviderPresenter
participant API as Provider API
participant Config as ConfigPresenter
participant DB as Provider DB (read-only)
Provider->>API: fetch models
alt success
API--)Provider: models
Provider->>Provider: parse/processModels()
Provider--)Caller: processedModels
else failure/empty
Provider->>Config: getDbProviderModels(providerId)
Config->>DB: read & map
Config--)Provider: dbModels
Provider--)Caller: dbModels (fallback)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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. Comment |
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/presenter/configPresenter/modelConfig.ts (1)
232-264: Legacy store entries become unreachable after normalizationOnce the provider DB path lowercases
providerId/modelId, pre-existing configs saved under mixed-case keys (e.g.OpenAI-_-GPT-4o) can no longer be retrieved: the first lookup uses the lowercased ID, and this fallback regenerates the exact same lowercased key. BecausegenerateCacheKeyis case-sensitive, the legacy entry is never found, so users silently lose their saved settings after upgrading. Please add a migration/lookup that either rewrites old keys to the normalized form or scans the cache/store for entries whose parsed IDs match case-insensitively before falling back to defaults.if (!normCached) { const fromStore = this.modelConfigStore.get(normKey) as IModelConfig | undefined if (fromStore) { normCached = fromStore this.memoryCache.set(normKey, fromStore) } } + if (!normCached && normProviderId && normModelId) { + const legacyKey = [...this.memoryCache.keys()].find((key) => { + if (this.isMetaKey(key)) return false + const { providerId: legacyProviderId, modelId: legacyModelId } = this.parseCacheKey(key) + return ( + legacyProviderId.toLowerCase() === normProviderId && + legacyModelId.toLowerCase() === normModelId + ) + }) + if (legacyKey) { + normCached = this.memoryCache.get(legacyKey) + if (!normCached) { + const legacyFromStore = this.modelConfigStore.get(legacyKey) as IModelConfig | undefined + if (legacyFromStore) { + normCached = legacyFromStore + this.memoryCache.set(legacyKey, legacyFromStore) + } + } + } + }
🧹 Nitpick comments (6)
scripts/fetch-provider-db.mjs (2)
20-26: Code duplication: ID validation logic exists in shared types.The regex patterns and validation functions for
providerIdandmodelIdare duplicated fromsrc/shared/types/model-db.ts(lines 61-70). This creates a maintenance burden and risk of divergence.Refactor to import and reuse the shared validation logic:
+import { isValidLowercaseProviderId, isValidLowercaseModelId } from '../src/shared/types/model-db.ts' + -const PROVIDER_ID_REGEX = /^[a-z0-9][a-z0-9-_]*$/ -const MODEL_ID_REGEX = /^[a-z0-9][a-z0-9\-_.:/]*$/ -const isValidLowercaseProviderId = (id) => - typeof id === 'string' && id === id.toLowerCase() && PROVIDER_ID_REGEX.test(id) -const isValidLowercaseModelId = (id) => - typeof id === 'string' && id === id.toLowerCase() && MODEL_ID_REGEX.test(id)
27-91: Consider extracting sanitizeAggregateJson to shared module.The
sanitizeAggregateJsonfunction implements business-critical sanitization logic. Sincesrc/shared/types/model-db.tsalready definessanitizeAggregate(mentioned in the AI summary), this script should reuse that shared implementation to avoid duplication and ensure consistency across build-time and runtime sanitization.Import and use the shared sanitization function:
+import { sanitizeAggregate } from '../src/shared/types/model-db.ts' + -function sanitizeAggregateJson(json) { - // ... 60+ lines of sanitization logic -} +// Use shared sanitization +const sanitized = sanitizeAggregate(json)src/shared/types/presenters/legacy.presenters.d.ts (1)
522-522: Improve type safety for getProviderDb return type.The return type
{ providers: Record<string, unknown> } | nullusesunknownwhich loses type information. Since the PR introduces aProviderAggregatetype insrc/shared/types/model-db.ts(mentioned in AI summary and relevant code snippets), the return type should reference that typed schema.Import and use the strongly-typed
ProviderAggregate:+import type { ProviderAggregate } from '../model-db' + - getProviderDb(): { providers: Record<string, unknown> } | null + getProviderDb(): ProviderAggregate | nullsrc/main/presenter/llmProviderPresenter/providers/ollamaProvider.ts (1)
77-92: Consider removing or correcting thedescriptionfield mapping.Line 86 explicitly sets
description: undefined, which is inconsistent with the pattern used in other provider fallbacks (e.g.,geminiProvider.tsandgithubCopilotProvider.tsin this PR) that omit the description field entirely when not available.If the Provider DB model has a
descriptionfield, consider using it:- description: undefined, + ...(m.description ? { description: m.description } : {}),Otherwise, simply omit the field to avoid storing
undefined:- description: undefined,This maintains consistency with other providers and avoids polluting the model metadata with explicit
undefinedvalues.src/main/presenter/configPresenter/providerDbLoader.ts (2)
43-57: Initialization flow is sound, but consider logging initialization failures.The lazy-load-then-background-refresh pattern is appropriate for startup performance. However, the silent catch at Line 56 may hide initialization errors that could help diagnose issues. Consider logging at least a warning when
refreshIfNeeded()fails, especially during the initial app lifecycle.Add minimal error logging:
// Background refresh if needed (npm 缓存风格) - this.refreshIfNeeded().catch(() => {}) + this.refreshIfNeeded().catch((err) => { + console.warn('[ProviderDbLoader] Background refresh failed during init:', err) + })
164-236: HTTP fetch implementation is robust, but add logging for non-304 failures.The implementation correctly handles:
- ETag-based caching (304 responses)
- Size guard to prevent memory issues
- Atomic writes via temp file
- Sanitization via
sanitizeAggregateHowever, silent error handling (Lines 180-184, 189-193, 198-201, 205-208, 231-233) may make debugging fetch failures difficult in production. Consider adding structured logging for non-304 HTTP errors and parse failures to help diagnose connectivity or data issues.
Add error logging for fetch failures:
if (!res.ok) { // Keep old cache + console.warn(`[ProviderDbLoader] Fetch failed: ${res.status} ${res.statusText}`) const meta = prevMeta ? { ...prevMeta, lastAttemptedAt: now } : undefined if (meta) this.writeMeta(meta) return }And for parse failures:
try { parsed = JSON.parse(text) } catch { + console.warn('[ProviderDbLoader] JSON parse failed for fetched data') const meta = prevMeta ? { ...prevMeta, lastAttemptedAt: now } : undefined if (meta) this.writeMeta(meta) return }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
package.json(1 hunks)resources/model-db/.gitkeep(1 hunks)scripts/fetch-provider-db.mjs(1 hunks)src/main/events.ts(1 hunks)src/main/presenter/configPresenter/index.ts(4 hunks)src/main/presenter/configPresenter/modelConfig.ts(4 hunks)src/main/presenter/configPresenter/modelDefaultSettings.ts(0 hunks)src/main/presenter/configPresenter/providerDbLoader.ts(1 hunks)src/main/presenter/llmProviderPresenter/providers/anthropicProvider.ts(1 hunks)src/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts(1 hunks)src/main/presenter/llmProviderPresenter/providers/geminiProvider.ts(1 hunks)src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts(1 hunks)src/main/presenter/llmProviderPresenter/providers/ollamaProvider.ts(1 hunks)src/renderer/src/events.ts(1 hunks)src/renderer/src/stores/settings.ts(4 hunks)src/shared/types/model-db.ts(1 hunks)src/shared/types/presenters/legacy.presenters.d.ts(2 hunks)test/main/presenter/providerDbModelConfig.test.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/presenter/configPresenter/modelDefaultSettings.ts
🧰 Additional context used
📓 Path-based instructions (27)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
**/*.{js,jsx,ts,tsx}: Use OxLint for JS/TS code; pre-commit hooks run lint-staged and typecheck
Use camelCase for variables and functions
Use PascalCase for types and classes
Use SCREAMING_SNAKE_CASE for constants
Files:
src/main/presenter/llmProviderPresenter/providers/ollamaProvider.tssrc/main/presenter/llmProviderPresenter/providers/anthropicProvider.tssrc/main/events.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.tssrc/renderer/src/events.tssrc/shared/types/presenters/legacy.presenters.d.tstest/main/presenter/providerDbModelConfig.test.tssrc/main/presenter/configPresenter/modelConfig.tssrc/main/presenter/llmProviderPresenter/providers/geminiProvider.tssrc/shared/types/model-db.tssrc/main/presenter/configPresenter/providerDbLoader.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/configPresenter/index.tssrc/renderer/src/stores/settings.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/providers/ollamaProvider.tssrc/main/presenter/llmProviderPresenter/providers/anthropicProvider.tssrc/main/events.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.tssrc/renderer/src/events.tssrc/main/presenter/configPresenter/modelConfig.tssrc/main/presenter/llmProviderPresenter/providers/geminiProvider.tssrc/main/presenter/configPresenter/providerDbLoader.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/configPresenter/index.tssrc/renderer/src/stores/settings.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Use Electron's built-in APIs for file system and native dialogs
Files:
src/main/presenter/llmProviderPresenter/providers/ollamaProvider.tssrc/main/presenter/llmProviderPresenter/providers/anthropicProvider.tssrc/main/events.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.tssrc/main/presenter/configPresenter/modelConfig.tssrc/main/presenter/llmProviderPresenter/providers/geminiProvider.tssrc/main/presenter/configPresenter/providerDbLoader.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/configPresenter/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪(如适用)、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/main/presenter/llmProviderPresenter/providers/ollamaProvider.tssrc/main/presenter/llmProviderPresenter/providers/anthropicProvider.tssrc/main/events.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.tssrc/renderer/src/events.tssrc/shared/types/presenters/legacy.presenters.d.tstest/main/presenter/providerDbModelConfig.test.tssrc/main/presenter/configPresenter/modelConfig.tssrc/main/presenter/llmProviderPresenter/providers/geminiProvider.tssrc/shared/types/model-db.tssrc/main/presenter/configPresenter/providerDbLoader.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/configPresenter/index.tssrc/renderer/src/stores/settings.ts
src/main/presenter/llmProviderPresenter/providers/*.ts
📄 CodeRabbit inference engine (.cursor/rules/llm-agent-loop.mdc)
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_start,tool_call_chunk,tool_call_end) in the standardized format.
Provider implementations should yield stop events with appropriatestop_reasonin the standardized format.
Provider implementations should yield error events in the standardized format...
Files:
src/main/presenter/llmProviderPresenter/providers/ollamaProvider.tssrc/main/presenter/llmProviderPresenter/providers/anthropicProvider.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.tssrc/main/presenter/llmProviderPresenter/providers/geminiProvider.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/llmProviderPresenter/providers/ollamaProvider.tssrc/main/presenter/llmProviderPresenter/providers/anthropicProvider.tssrc/main/events.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.tssrc/main/presenter/configPresenter/modelConfig.tssrc/main/presenter/llmProviderPresenter/providers/geminiProvider.tssrc/main/presenter/configPresenter/providerDbLoader.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/configPresenter/index.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/main/presenter/llmProviderPresenter/providers/ollamaProvider.tssrc/main/presenter/llmProviderPresenter/providers/anthropicProvider.tssrc/main/events.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.tssrc/renderer/src/events.tssrc/shared/types/presenters/legacy.presenters.d.tstest/main/presenter/providerDbModelConfig.test.tssrc/main/presenter/configPresenter/modelConfig.tssrc/main/presenter/llmProviderPresenter/providers/geminiProvider.tssrc/shared/types/model-db.tssrc/main/presenter/configPresenter/providerDbLoader.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/configPresenter/index.tssrc/renderer/src/stores/settings.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Files:
src/main/presenter/llmProviderPresenter/providers/ollamaProvider.tssrc/main/presenter/llmProviderPresenter/providers/anthropicProvider.tssrc/main/events.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.tssrc/renderer/src/events.tssrc/shared/types/presenters/legacy.presenters.d.tstest/main/presenter/providerDbModelConfig.test.tssrc/main/presenter/configPresenter/modelConfig.tssrc/main/presenter/llmProviderPresenter/providers/geminiProvider.tssrc/shared/types/model-db.tssrc/main/presenter/configPresenter/providerDbLoader.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/configPresenter/index.tssrc/renderer/src/stores/settings.ts
src/main/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all Electron main-process code under src/main/
Files:
src/main/presenter/llmProviderPresenter/providers/ollamaProvider.tssrc/main/presenter/llmProviderPresenter/providers/anthropicProvider.tssrc/main/events.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.tssrc/main/presenter/configPresenter/modelConfig.tssrc/main/presenter/llmProviderPresenter/providers/geminiProvider.tssrc/main/presenter/configPresenter/providerDbLoader.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/configPresenter/index.ts
src/main/presenter/**
📄 CodeRabbit inference engine (AGENTS.md)
src/main/presenter/**: Organize main-process presenters under src/main/presenter/ (Window/Tab/Thread/Mcp/Config/LLMProvider)
Follow the Presenter pattern for main-process modules
Files:
src/main/presenter/llmProviderPresenter/providers/ollamaProvider.tssrc/main/presenter/llmProviderPresenter/providers/anthropicProvider.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.tssrc/main/presenter/configPresenter/modelConfig.tssrc/main/presenter/llmProviderPresenter/providers/geminiProvider.tssrc/main/presenter/configPresenter/providerDbLoader.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/configPresenter/index.ts
**/*.{js,jsx,ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting: single quotes, no semicolons, max width 100
Files:
src/main/presenter/llmProviderPresenter/providers/ollamaProvider.tssrc/main/presenter/llmProviderPresenter/providers/anthropicProvider.tssrc/main/events.tssrc/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.tssrc/renderer/src/events.tssrc/shared/types/presenters/legacy.presenters.d.tstest/main/presenter/providerDbModelConfig.test.tssrc/main/presenter/configPresenter/modelConfig.tssrc/main/presenter/llmProviderPresenter/providers/geminiProvider.tssrc/shared/types/model-db.tssrc/main/presenter/configPresenter/providerDbLoader.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/configPresenter/index.tssrc/renderer/src/stores/settings.ts
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/events.tssrc/renderer/src/stores/settings.ts
src/renderer/**/*.{vue,ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
渲染进程代码放在
src/renderer
Files:
src/renderer/src/events.tssrc/renderer/src/stores/settings.ts
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/events.tssrc/renderer/src/stores/settings.ts
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/events.tssrc/renderer/src/stores/settings.ts
src/renderer/**/*.{vue,ts}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
Implement lazy loading for routes and components.
Files:
src/renderer/src/events.tssrc/renderer/src/stores/settings.ts
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.Use Pinia for frontend state management (do not introduce alternative state libraries)
Files:
src/renderer/src/events.tssrc/renderer/src/stores/settings.ts
src/renderer/src/**
📄 CodeRabbit inference engine (AGENTS.md)
Put application code for the Vue app under src/renderer/src (components, stores, views, i18n, lib)
Files:
src/renderer/src/events.tssrc/renderer/src/stores/settings.ts
src/renderer/src/**/*.{vue,ts}
📄 CodeRabbit inference engine (AGENTS.md)
All user-facing strings in the renderer must use vue-i18n keys defined in src/renderer/src/i18n
Files:
src/renderer/src/events.tssrc/renderer/src/stores/settings.ts
src/shared/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
共享类型定义放在
shared目录
Files:
src/shared/types/presenters/legacy.presenters.d.tssrc/shared/types/model-db.ts
src/shared/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place shared types, utilities, constants, and IPC contract definitions under src/shared/
Files:
src/shared/types/presenters/legacy.presenters.d.tssrc/shared/types/model-db.ts
src/shared/**
📄 CodeRabbit inference engine (AGENTS.md)
Store shared TypeScript types/utilities in src/shared/
Files:
src/shared/types/presenters/legacy.presenters.d.tssrc/shared/types/model-db.ts
test/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Place unit and integration tests under the test/ directory mirroring project structure
Files:
test/main/presenter/providerDbModelConfig.test.ts
test/{main,renderer}/**
📄 CodeRabbit inference engine (AGENTS.md)
Mirror source structure in tests under test/main/** and test/renderer/** (with setup files)
Files:
test/main/presenter/providerDbModelConfig.test.ts
test/{main,renderer}/**/*.{test,spec}.ts
📄 CodeRabbit inference engine (AGENTS.md)
test/{main,renderer}/**/*.{test,spec}.ts: Name test files with *.test.ts or *.spec.ts
Write tests with Vitest (jsdom) and Vue Test Utils
Files:
test/main/presenter/providerDbModelConfig.test.ts
scripts/**
📄 CodeRabbit inference engine (AGENTS.md)
Place build/signing/installer/runtime and commit-related scripts in scripts/
Files:
scripts/fetch-provider-db.mjs
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
🧠 Learnings (2)
📚 Learning: 2025-09-06T03:07:23.817Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T03:07:23.817Z
Learning: Applies to src/main/presenter/configPresenter/providers.ts : Add provider configuration entries in src/main/presenter/configPresenter/providers.ts
Applied to files:
src/shared/types/presenters/legacy.presenters.d.tstest/main/presenter/providerDbModelConfig.test.tssrc/main/presenter/configPresenter/modelConfig.tssrc/main/presenter/configPresenter/providerDbLoader.tssrc/main/presenter/configPresenter/index.ts
📚 Learning: 2025-09-06T03:07:23.817Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T03:07:23.817Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : New LLM providers must be added under src/main/presenter/llmProviderPresenter/providers/ as separate files
Applied to files:
src/main/presenter/configPresenter/providerDbLoader.ts
🧬 Code graph analysis (10)
src/main/events.ts (1)
src/renderer/src/events.ts (1)
PROVIDER_DB_EVENTS(113-116)
src/renderer/src/events.ts (1)
src/main/events.ts (1)
PROVIDER_DB_EVENTS(41-44)
src/shared/types/presenters/legacy.presenters.d.ts (1)
src/shared/types/presenters/llmprovider.presenter.d.ts (1)
RENDERER_MODEL_META(10-25)
test/main/presenter/providerDbModelConfig.test.ts (1)
src/main/presenter/configPresenter/modelConfig.ts (1)
ModelConfigHelper(18-537)
scripts/fetch-provider-db.mjs (1)
src/shared/types/model-db.ts (2)
isValidLowercaseProviderId(62-66)isValidLowercaseModelId(67-71)
src/main/presenter/configPresenter/modelConfig.ts (3)
src/shared/types/presenters/legacy.presenters.d.ts (3)
ModelConfig(132-150)ModelConfigSource(130-130)IModelConfig(152-157)src/main/presenter/configPresenter/providerDbLoader.ts (1)
providerDbLoader(240-240)src/shared/types/model-db.ts (1)
isImageInputSupported(54-57)
src/shared/types/model-db.ts (1)
scripts/fetch-provider-db.mjs (14)
PROVIDER_ID_REGEX(20-20)MODEL_ID_REGEX(21-21)isValidLowercaseProviderId(22-23)isValidLowercaseModelId(24-25)out(31-31)pid(34-34)sanitizedModels(38-38)mid(41-41)limit(44-44)rlimit(43-43)modalities(53-53)rmods(54-54)inp(56-56)envArr(78-78)
src/main/presenter/configPresenter/providerDbLoader.ts (4)
src/shared/types/model-db.ts (4)
ProviderAggregate(50-50)ProviderEntry(44-44)ProviderModel(32-32)sanitizeAggregate(104-190)test/mocks/electron.ts (1)
app(2-10)src/main/eventbus.ts (1)
eventBus(151-151)src/main/events.ts (1)
PROVIDER_DB_EVENTS(41-44)
src/main/presenter/configPresenter/index.ts (4)
src/main/presenter/configPresenter/providerDbLoader.ts (1)
providerDbLoader(240-240)src/shared/types/model-db.ts (1)
ProviderAggregate(50-50)src/shared/types/presenters/legacy.presenters.d.ts (2)
RENDERER_MODEL_META(531-545)ModelConfig(132-150)src/shared/types/presenters/llmprovider.presenter.d.ts (1)
RENDERER_MODEL_META(10-25)
src/renderer/src/stores/settings.ts (2)
src/shared/types/presenters/legacy.presenters.d.ts (1)
RENDERER_MODEL_META(531-545)src/renderer/src/events.ts (1)
PROVIDER_DB_EVENTS(113-116)
⏰ 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 (22)
scripts/fetch-provider-db.mjs (4)
120-124: Size limit validation is appropriate.The 5MB size limit (line 120) is a reasonable safeguard against malformed or malicious responses. The fallback logic (line 122-123) is correct.
135-140: Error handling for sanitization failure is correct.If sanitization returns
null(no valid providers), the script logs an error and exits non-zero if no existing snapshot is present. This is appropriate behavior.
142-155: Atomic write pattern is correct.Writing to a temporary file and then renaming (lines 143-150) is a correct atomic-write pattern. The cross-platform fallback (lines 147-149) handles Windows rename limitations.
110-118: No CI/CD or build-script references found
Searched the entire repository (excluding node_modules) for “fetch-provider-db” and “providers.json” and found no matches, so no workflows depend on distinguishing stale snapshots. No changes required unless you add monitoring or pipeline logic that needs to detect a fallback.resources/model-db/.gitkeep (1)
1-1: LGTM: Standard .gitkeep pattern.This empty file correctly preserves the
resources/model-db/directory in version control.src/shared/types/presenters/legacy.presenters.d.ts (1)
386-386: LGTM: Method signature is type-safe.The
getDbProviderModelsmethod signature correctly returnsRENDERER_MODEL_META[], which is a well-defined type in this file (lines 531-545).src/renderer/src/events.ts (1)
113-116: LGTM: Event constants are consistent with main process.The
PROVIDER_DB_EVENTScorrectly mirrors the event names fromsrc/main/events.ts(lines 40-43). Event naming follows the established pattern (provider-db:loaded,provider-db:updated).package.json (1)
14-14: Missing‐file fallback is handled—no changes needed.
providerDbLoader.initialize()andgetDb()both fallback to loadingresources/model-db/providers.jsonwhen the fetched file is missing, so runningpnpm devwithout a prior build won’t error as long as that built-in file exists.src/main/events.ts (1)
40-44: LGTM!The new
PROVIDER_DB_EVENTSdomain is well-defined with clear event names and follows the existing event naming conventions. The comments appropriately distinguish between initial load (LOADED) and remote refresh (UPDATED), and the events are correctly mirrored in the renderer for cross-process communication.src/main/presenter/llmProviderPresenter/providers/geminiProvider.ts (1)
252-266: LGTM!The fallback logic correctly migrates from the static
GEMINI_MODELSlist to the centralized Provider DB. The mapping applies sensible defaults (group: 'default', boolean flags default tofalse) and conditionally includes thetypefield only when present. This aligns with the PR's goal of centralizing model metadata in the Provider DB.src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts (1)
223-240: Verify that exclusive Provider DB usage is correct for GitHub Copilot.The
fetchProviderModelsmethod now exclusively uses the Provider DB as the "authoritative source" without any fallback to an API fetch. This differs from other providers in this PR (e.g., Gemini, Ollama) that fall back to the DB only when the API fetch fails.Please confirm this is the intended design for GitHub Copilot. If GitHub Copilot's API does not expose a model listing endpoint, this approach is appropriate. Otherwise, consider whether a primary API fetch with DB fallback would be more consistent with other providers.
src/renderer/src/stores/settings.ts (3)
8-8: LGTM!Correct import of the new
PROVIDER_DB_EVENTSevent domain, used in the listener setup at lines 738-744.
456-540: LGTM!The refactored
refreshStandardModelscorrectly implements a two-step data source approach: primary fetch from the centralized Provider DB, with fallback tollmP.getModelListif the DB is empty. The addition of|| 'default'on line 472 ensures thegroupfield always has a value, preventing potential issues with missing groups.
738-744: LGTM!The Provider DB event listeners are correctly implemented. Both
PROVIDER_DB_EVENTS.UPDATEDandPROVIDER_DB_EVENTS.LOADEDappropriately trigger a full model refresh viarefreshAllModels(), ensuring the UI stays in sync with DB changes. The comment clearly documents the intent.src/main/presenter/llmProviderPresenter/providers/anthropicProvider.ts (1)
164-183: LGTM! DB-backed fallback correctly implemented.The migration from static fallback to Provider DB-backed fallback is clean and complete. The mapping covers all required MODEL_META fields with sensible defaults (group defaults to 'default', boolean flags default to false). The optional type field is conditionally spread, avoiding unnecessary properties.
test/main/presenter/providerDbModelConfig.test.ts (1)
1-149: Comprehensive test coverage for Provider DB integration.The test suite effectively validates:
- Strict provider/model matching with full config fields
- Partial fallback logic for missing limit fields
- Safe defaults when no provider is specified
- User config precedence and persistence across restarts/version bumps
- Case-insensitive DB lookups
The mock implementations (electron-store and providerDbLoader) are well-structured and reusable. The test scenarios align with the PR's goal of migrating to DB-driven model configuration.
src/main/presenter/configPresenter/providerDbLoader.ts (1)
91-117: Built-in fallback candidate paths cover dev and production scenarios.The fallback path resolution checks both packaged (
process.resourcesPath) and dev (process.cwd()) locations, ensuring the loader can find built-in data in both environments. This is a solid approach for graceful degradation when remote fetch fails.src/main/presenter/configPresenter/index.ts (4)
34-35: Provider DB initialization correctly integrated.The loader is initialized during ConfigPresenter construction, ensuring the DB is available for subsequent operations. The silent catch is consistent with the loader's design of graceful fallback to built-in data.
Also applies to: 176-177
207-209: Read-only DB access correctly exposed.Exposing
getProviderDb()allows other modules to query the aggregate without mutation, maintaining encapsulation.
640-659: DB model mapping is comprehensive and correctly handles optional fields.The mapping to
RENDERER_MODEL_METAcovers all required fields:
- Correctly falls back to
display_name || name || idfor the name field- Properly handles limit fallbacks (8192 context, 4096 output)
- Detects vision capability by checking
modalities.inputfor 'image'- Type-casts
tool_callandreasoningto booleanThe implementation is consistent with the test expectations in
providerDbModelConfig.test.ts.
667-669: Defaults forcontextLengthandtemperatureupdated intentionally Tests acrossproviderDbModelConfig.test.tsandmodelConfig.test.tsconfirm the new defaults ofcontextLength: 8192andtemperature: 0.6, matching the intended behavior.src/main/presenter/llmProviderPresenter/providers/awsBedrockProvider.ts (1)
97-116: Confirmamazon-bedrockmatchesAWS_BEDROCK_PROVIDER.id
Verify that the literal'amazon-bedrock'passed togetDbProviderModelsaligns withthis.provider.iddefined inAWS_BEDROCK_PROVIDER(from@shared/presenter).
|
@codex review this |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
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: 9
♻️ Duplicate comments (1)
src/main/presenter/configPresenter/providerDbLoader.ts (1)
91-102: Verify built-in fallback works in packaged apps.A previous review comment flagged that the built-in fallback might not work in packaged apps due to asar bundling. However,
app.getAppPath()should return the correct path (e.g.,/path/to/app.asar) and Electron's patchedfsmodule can read from inside asar files. Please verify that this approach works correctly in production builds, especially if the file is bundled insideapp.asar.Run the following script to check if the built-in provider DB path exists in a packaged build:
#!/bin/bash # Description: Verify the built-in provider DB path construction and file existence logic. # This script checks if the path construction in loadFromBuiltIn() is correct. # Search for any comments or documentation about asar bundling and the provider DB path rg -n -C5 'app\.getAppPath|resources/model-db|app\.asar' --type=ts # Find any other references to the built-in provider DB path rg -n -C3 'providers\.json' --type=ts
🧹 Nitpick comments (1)
src/main/presenter/configPresenter/providerDbLoader.ts (1)
78-89: Log cache loading errors for better observability.The empty
catchblock silently swallows errors when reading or parsing the cache file. While returningnullis a safe fallback, logging the error would help diagnose issues such as corrupted cache files or permission problems.Apply this diff to add error logging:
private loadFromCache(): ProviderAggregate | null { try { if (!fs.existsSync(this.cacheFilePath)) return null const raw = fs.readFileSync(this.cacheFilePath, 'utf-8') const parsed = JSON.parse(raw) const sanitized = sanitizeAggregate(parsed) if (!sanitized) return null return sanitized - } catch { + } catch (err) { + console.warn('[ProviderDbLoader] Failed to load cache', { + cacheFilePath: this.cacheFilePath, + error: err instanceof Error ? err.message : String(err) + }) return null } }As per coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/presenter/configPresenter/providerDbLoader.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
**/*.{js,jsx,ts,tsx}: Use OxLint for JS/TS code; pre-commit hooks run lint-staged and typecheck
Use camelCase for variables and functions
Use PascalCase for types and classes
Use SCREAMING_SNAKE_CASE for constants
Files:
src/main/presenter/configPresenter/providerDbLoader.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/configPresenter/providerDbLoader.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Use Electron's built-in APIs for file system and native dialogs
Files:
src/main/presenter/configPresenter/providerDbLoader.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪(如适用)、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/main/presenter/configPresenter/providerDbLoader.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/configPresenter/providerDbLoader.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/main/presenter/configPresenter/providerDbLoader.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Files:
src/main/presenter/configPresenter/providerDbLoader.ts
src/main/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all Electron main-process code under src/main/
Files:
src/main/presenter/configPresenter/providerDbLoader.ts
src/main/presenter/**
📄 CodeRabbit inference engine (AGENTS.md)
src/main/presenter/**: Organize main-process presenters under src/main/presenter/ (Window/Tab/Thread/Mcp/Config/LLMProvider)
Follow the Presenter pattern for main-process modules
Files:
src/main/presenter/configPresenter/providerDbLoader.ts
**/*.{js,jsx,ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting: single quotes, no semicolons, max width 100
Files:
src/main/presenter/configPresenter/providerDbLoader.ts
🧠 Learnings (2)
📚 Learning: 2025-09-06T03:07:23.817Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T03:07:23.817Z
Learning: Applies to src/main/presenter/configPresenter/providers.ts : Add provider configuration entries in src/main/presenter/configPresenter/providers.ts
Applied to files:
src/main/presenter/configPresenter/providerDbLoader.ts
📚 Learning: 2025-09-06T03:07:23.817Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T03:07:23.817Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : New LLM providers must be added under src/main/presenter/llmProviderPresenter/providers/ as separate files
Applied to files:
src/main/presenter/configPresenter/providerDbLoader.ts
🧬 Code graph analysis (1)
src/main/presenter/configPresenter/providerDbLoader.ts (3)
src/shared/types/model-db.ts (4)
ProviderAggregate(50-50)ProviderEntry(44-44)ProviderModel(32-32)sanitizeAggregate(104-190)src/main/eventbus.ts (1)
eventBus(151-151)src/main/events.ts (1)
PROVIDER_DB_EVENTS(41-44)
⏰ 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)
|
@codex review |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
migrate model config to external DB
Summary by CodeRabbit
New Features
Chores
Tests